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

[ENH] Edit Domain: Merge categorical values #3725

Merged
merged 8 commits into from Apr 12, 2019

Conversation

Projects
None yet
2 participants
@ales-erjavec
Copy link
Contributor

commented Apr 5, 2019

Issue

Implements gh-3371

Also fixes and assertion error when two values are renamed to the same value.

Traceback (most recent call last):
  File "/Applications/Orange3.app/Contents/Frameworks/Python.framework/Versions/3.6/lib/python3.6/site-packages/Orange/widgets/data/oweditdomain.py", line 1165, in commit
    var = apply_transform(v, tr)
  File "/Applications/Orange3.app/Contents/Frameworks/Python.framework/Versions/3.6/lib/python3.6/functools.py", line 803, in wrapper
    return dispatch(args[0].__class__)(*args, **kw)
  File "/Applications/Orange3.app/Contents/Frameworks/Python.framework/Versions/3.6/lib/python3.6/site-packages/Orange/widgets/data/oweditdomain.py", line 1430, in apply_transform_discete
    dest_codes = positions(dest_values)
  File "/Applications/Orange3.app/Contents/Frameworks/Python.framework/Versions/3.6/lib/python3.6/site-packages/Orange/widgets/data/oweditdomain.py", line 1427, in positions
    assert len(rval) == len(values)
AssertionError
Description of changes

Implement merging of categorical values.

Merging can be achieved by simply renaming individual values to the same name, or by selecting multiple values and pressing the merge button (shortcut ctrl + =) and entering/selecting a name in an editable combo box.

Includes
  • Code changes
  • Tests
  • Documentation

@ales-erjavec ales-erjavec changed the title [ENH] Edit Domain: Merge categorical features [ENH] Edit Domain: Merge categorical values Apr 5, 2019

@codecov

This comment has been minimized.

Copy link

commented Apr 5, 2019

Codecov Report

Merging #3725 into master will increase coverage by 0.27%.
The diff coverage is 96.48%.

@@            Coverage Diff             @@
##           master    #3725      +/-   ##
==========================================
+ Coverage   84.53%   84.81%   +0.27%     
==========================================
  Files         373      374       +1     
  Lines       68471    68749     +278     
==========================================
+ Hits        57884    58307     +423     
+ Misses      10587    10442     -145

ales-erjavec added some commits Apr 5, 2019

oweditdomain: Refactor of the CountedListModel
... to count values within renamed values only and not added/dropped.

import numpy as np

import Orange.data

from Orange.preprocess.transformation import Identity, Lookup
from Orange.widgets import widget, gui, settings
from Orange.widgets.data.owconcatenate import unique

This comment has been minimized.

Copy link
@janezd

janezd Apr 10, 2019

Contributor

I'd avoid importing functions from widget modules if possible because it messes up dependencies and complicates future refactorings. I think we'd better treat all functions in widget modules as private (even if not named as such), just as we treat widget classes as final.

Do we have a better place for functions like unique?

Perhaps better, isn't unique(s) same as iter(dict.fromkeys(s)) (formally from Python 3.7, de facto already in 3.6). In below code, it's used as list(unique(s)), which is, I suppose, the same as list(dict.fromkeys(s)).

@janezd janezd self-assigned this Apr 12, 2019

@janezd janezd merged commit 761dd15 into biolab:master Apr 12, 2019

5 checks passed

codecov/patch 96.48% of diff hit (target 95%)
Details
codecov/project 84.81% (+0.27%) compared to 399c4ef
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.