Skip to content

[FIX] Scatter Plot: Update axis labels when discrete values change - #7336

Open
raashish1601 wants to merge 1 commit into
biolab:masterfrom
raashish1601:fix/7308-scatter-axis-labels
Open

raashish1601 wants to merge 1 commit into
biolab:masterfrom
raashish1601:fix/7308-scatter-axis-labels

Conversation

@raashish1601

Copy link
Copy Markdown
Issue

Fixes #7308

Description of changes

When new data has the same values in X but a discrete variable's values are renamed (e.g. the steps in the issue: edit the category names in the file and press Reload), Scatter Plot keeps the old category labels on the axis while the legend shows the new ones.

OWDataProjectionWidget.set_data only marks the domain as changed when domain.checksum() differs, and Scatter Plot updates its axes only in that case. The checksum is the domain's hash, and a variable's hash is built from its name, type and compute value, not its values. So renaming the values leaves _domain_invalidated false, the data is not invalidated either (the arrays are equal), and update_axes is never called.

The domain is now also treated as changed when the values of its discrete variables differ.

Tests: test_discrete_axis_values_changed sends data with a discrete variable valued ("1", "2"), then the same table with values ("a", "b"), and checks the x-axis ticks. It fails without the change (the ticks stay 1, 2) and passes with it. test_owscatterplot.py, test_owlinearprojection.py, test_owradviz.py, test_owfreeviz.py and test_owmds.py pass locally (Windows, against the Orange 3.40 wheel with this patch applied, apart from one test in the master copy of test_owscatterplot.py that needs a newer graph than 3.40 has).

Includes
  • Code changes
  • Tests
  • Documentation

@CLAassistant

CLAassistant commented Oct 7, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@janezd janezd self-assigned this Oct 9, 2026
@janezd janezd added this to the 3.41 milestone Oct 9, 2026
@codecov

codecov Bot commented Oct 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.97%. Comparing base (eea7a75) to head (cd44b04).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7336      +/-   ##
==========================================
- Coverage   88.98%   88.97%   -0.01%     
==========================================
  Files         337      337              
  Lines       74598    74601       +3     
==========================================
+ Hits        66378    66379       +1     
- Misses       8220     8222       +2     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@raashish1601

Copy link
Copy Markdown
Author

The "Scientific Python nightly wheels" failure is unrelated to this change: the nightly pyqtgraph turned LegendItem.layout into a method, so hundreds of widget tests error out. Master fails the same way (https://github.com/biolab/orange3/actions/runs/37914329734).

@janezd

janezd commented Oct 10, 2026

Copy link
Copy Markdown
Contributor

@raashish1601, thanks. The failed test is indeed caused by an (irresponsible) change in pyqtgraph and would be patched in #7329.

The diagnosis and the fix is essentially correct, but I think that it would be better to put the change deeper.

The genesis of the problem is such.

  • Scatter plot uses Domain.checksum to check whether the domain has changed.
  • Domain.checksum returns hash(domain).
  • Hash of a domain is composed of hashes of variables.
  • Hash of a variable takes into account its name, type and compute value -- but not a list of values of discrete variable.

We can fix the code in three places:

  • DiscreteVariable.__hash__ should include values. Yet, this hash is used all around the code, so the change might break something. (Or, possibly, fix something.) This would be a proper fix.
  • Domain.checksum could call Domain's hash and add values. This is strange, but safe: it can't break anything (except possibly in add-ons) because this method is used called exclusively from the scatter plot.
  • Current PR, which changes the scatter plot by checking discrete values. This is safest and not strange, but partial -- essentially a very late patch.

I would lean towards DiscreteVariable.__hash__ and hope that tests will discover any code that requires the current (invalid) implementation.

@markotoplak, @VesnaT, what do you think?

@raashish1601

Copy link
Copy Markdown
Author

Thanks for the explanation. I'm happy to move the fix into DiscreteVariable.__hash__ (with a test there) if that's the direction you pick, and drop the scatter plot change. I'll wait for the decision.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Scatter Plot x-Axis confuses the categorical variable

3 participants