Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Discrete variable: remove ordered attribute #4818

Merged
merged 1 commit into from
Jun 12, 2020

Conversation

PrimozGodec
Copy link
Contributor

@PrimozGodec PrimozGodec commented May 26, 2020

Issue

As we already discussed in #4793 ordered attribute from the Discrete variable is not respected in most of the widgets anymore (except Feature Statistics), so it can be removed. Order defined in values tuple should be respected in any widget which needs order.

Description of changes
  • The ordered attribute is deprecated and will stay so until 3.28.0 then it can be removed.
  • Checkbox which sets ordered attribute in the edit domain is removed
  • Feature statistic will never show min and max value for the discrete variable (before it happened in a case when ordered attribute was True)
  • When saving data always write the order of values for discrete in the header (except in the case with only one value)
Includes
  • Code changes
  • Tests

@PrimozGodec PrimozGodec changed the title Discrete variable: remove ordered attribute [WIP] Discrete variable: remove ordered attribute May 26, 2020
@PrimozGodec PrimozGodec force-pushed the remove-ordered branch 3 times, most recently from f2c611e to 029071f Compare May 29, 2020 09:30
@codecov
Copy link

codecov bot commented May 29, 2020

Codecov Report

Merging #4818 into master will decrease coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #4818      +/-   ##
==========================================
- Coverage   84.19%   84.18%   -0.02%     
==========================================
  Files         282      277       -5     
  Lines       57355    56472     -883     
==========================================
- Hits        48292    47541     -751     
+ Misses       9063     8931     -132     

@PrimozGodec PrimozGodec changed the title [WIP] Discrete variable: remove ordered attribute Discrete variable: remove ordered attribute May 29, 2020
@janezd janezd self-assigned this Jun 5, 2020
def __init__(
self, name="", values=(), compute_value=None, sparse=False,
**kwargs
):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit dangerous: any call that passes compute_value as non-keyword argument will (possibly silently) fail. What about keeping ordered as an argument, but changing its default to None and the test if "ordered" in kwargs: to if ordered is not None?

Though: this is unlikely and, besides, such calls won't be fixed until the argument is actually removed and the calls will actually fail. We ignore warnings, including those about deprecations.

Copy link
Contributor Author

@PrimozGodec PrimozGodec Jun 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right. Didn't think about this possibility. This solution was to avoid problems with __repr__ but now I fixed it differently.

Orange/tests/test_filter.py Outdated Show resolved Hide resolved
@PrimozGodec PrimozGodec force-pushed the remove-ordered branch 7 times, most recently from 340d71c to 7c222a2 Compare June 11, 2020 16:51
OWMarkerGenes: settings migration and backward compatibility
@janezd janezd merged commit 1ad65b9 into biolab:master Jun 12, 2020
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.

2 participants