-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
[FIX] oweditdomain: Hide "categories mapping" warning on change #6865
[FIX] oweditdomain: Hide "categories mapping" warning on change #6865
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6865 +/- ##
==========================================
- Coverage 88.26% 88.20% -0.07%
==========================================
Files 326 326
Lines 71142 71264 +122
==========================================
+ Hits 62797 62861 +64
- Misses 8345 8403 +58 |
37e6d9e
to
369d6d5
Compare
369d6d5
to
27280fd
Compare
I apologise but I don't properly see the difference after the fix. I do like the highlight of the changed variable, although it would be better probably to use yellow as a background highlight (yellow letters are difficult to see on white). But the warning behaviour is still identical for me (or am I missing something)? The user says <"A" if 0 else "B">. Then she changes this to <"C" if 0 else "B">. The warning says the mapping doesn't apply to the current input. But it does (i.e. should be updated in the background). The only thing that has changed was how I want to name my value ("C" instead of "A")? So in my opinion, there should be no warning. Even the widget Edit Domain shows the mapping worked (it changes "A" to "C"). It is entirely possible I don't understand the issue here. |
Orange/widgets/data/oweditdomain.py
Outdated
option.palette.setBrush(QPalette.HighlightedText, QBrush(Qt.red)) | ||
set_color(option.palette, Qt.red) | ||
elif warnings_: | ||
set_color(option.palette, Qt.yellow) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with @ajdapretnar (if I understand her correctly): it should be possible to change the name of a category on the input of Edit Domain, or even add new categories, and this should be updated in the background. Actually, in current Orange 3.37.0 it is updated in the background - and everything works fine, even if new categories are added: the changes appear in the output of Edit Domain, as intended (I just checked). There just shouldn't be a warning! |
Issue
Fixes: #6861
Description of changes
Includes