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] Reorganize continuous palettes #4305

Merged
merged 22 commits into from
Feb 7, 2020

Conversation

janezd
Copy link
Contributor

@janezd janezd commented Dec 30, 2019

Issue

Fixes #1924

Description of changes
  • Takes a subset of palettes from colorcet
  • Implements classes for DiscretePalette, ContinuousPalette and BinnedContinuousPalette
  • Implements a model and a combo box for choosing palettes
  • Ports core widgets to new palettes and marks colorbrewer and colorpalette as obsolete
  • Replaces gradient picker in the Color widget with predefined palettes
  • Eliminates gamma from heatmap because; let the user set the range instead

To fix:

  • SOM: Color by continuous and select 'Show pie charts'. Error is thrown.
  • Tree Viewer (with housing): Color by mean value. Error is thrown.
  • Pythagorean Tree with node color Mean: legend is wrong.
  • Eliminate centering checkbox in the heatmap; center when the palette is diverging

To consider for the future (rather than here):

  • Legends. Provide methods for drawing legends. Some can be ported from existing widgets. Consider using module owlegend. We should decide whether all widgets should eventually use that module for drawing legends.
  • Utility functions Move Scale and some other methods from owscatterplotgraph and possibly owsom to some utils module so they can be shared with other widgets.
Includes
  • Code changes
  • Tests
  • Documentation

@janezd janezd changed the title Reorganize continuous palettes [RFC] [WIP] Reorganize continuous palettes Dec 30, 2019
@ajdapretnar
Copy link
Contributor

This looks much better than before. +1 for the PR!

@ajdapretnar
Copy link
Contributor

2 bugs.

  1. File - Tree - Tree Viewer (select Target class: iris setosa)
-------------------------- AttributeError Exception ---------------------------
Traceback (most recent call last):
  File "/Users/ajda/orange/orange3/Orange/widgets/visualize/owtreeviewer.py", line 235, in color_changed
    self.toggle_node_color_cls()
  File "/Users/ajda/orange/orange3/Orange/widgets/visualize/owtreeviewer.py", line 387, in toggle_node_color_cls
    color = colors[self.target_class_index - 1].lighter(
AttributeError: 'numpy.ndarray' object has no attribute 'lighter'
-------------------------------------------------------------------------------
  1. File - SOM.
---------------------------- ValueError Exception -----------------------------
Traceback (most recent call last):
  File "/Users/ajda/orange/orange-canvas-core/orangecanvas/scheme/signalmanager.py", line 936, in __process_next
    if self.__process_next_helper(use_max_active=True):
  File "/Users/ajda/orange/orange-canvas-core/orangecanvas/scheme/signalmanager.py", line 974, in __process_next_helper
    self.process_node(selected_node)
  File "/Users/ajda/orange/orange-canvas-core/orangecanvas/scheme/signalmanager.py", line 605, in process_node
    self.send_to_node(node, signals_in)
  File "/Users/ajda/orange/orange-widget-base/orangewidget/workflow/widgetsscheme.py", line 792, in send_to_node
    self.process_signals_for_widget(node, widget, signals)
  File "/Users/ajda/orange/orange-widget-base/orangewidget/workflow/widgetsscheme.py", line 826, in process_signals_for_widget
    handler(*args)
  File "/Users/ajda/orange/orange3/Orange/widgets/unsupervised/owsom.py", line 354, in set_data
    self.create_legend()
  File "/Users/ajda/orange/orange3/Orange/widgets/unsupervised/owsom.py", line 871, in create_legend
    Qt.gray, color))
  File "/Users/ajda/orange/orange3/Orange/widgets/visualize/utils/__init__.py", line 679, in __init__
    if brush_color:
ValueError: The truth value of an array with more than one element is ambiguous. Use a.any() or a.all()
-------------------------------------------------------------------------------

@janezd janezd force-pushed the continuous-palettes branch 4 times, most recently from cbe2051 to e21cde0 Compare January 24, 2020 15:24
@codecov
Copy link

codecov bot commented Jan 24, 2020

Codecov Report

Merging #4305 into master will increase coverage by 0.24%.
The diff coverage is 98.98%.

@@            Coverage Diff             @@
##           master    #4305      +/-   ##
==========================================
+ Coverage   87.19%   87.43%   +0.24%     
==========================================
  Files         401      405       +4     
  Lines       72938    74061    +1123     
==========================================
+ Hits        63595    64757    +1162     
+ Misses       9343     9304      -39

@janezd janezd force-pushed the continuous-palettes branch 3 times, most recently from dfc97b1 to 58d4ed2 Compare January 26, 2020 21:20
@ajdapretnar
Copy link
Contributor

Issues:

  • Color does not propagate changes at all times (use Copy to all option).
  • Distance Map does not show selection in an obvious way. Selections are now way too similar to the palette, so they are practically invisible (especially when you use a reddish color palette).
  • SOM: Color by continuous and select 'Show pie charts'. Error is thrown.
  • Tree Viewer (with housing): Color by mean value. Error is thrown.
  • Pythagorean Tree with node color Mean: legend is wrong.

Don't honestly know what to check in Paint Data, Calibration Plot, Lift Curve and ROC Analysis. Or Venn Diagram.

@janezd
Copy link
Contributor Author

janezd commented Jan 31, 2020

Color does not propagate changes at all times (use Copy to all option).

I don't see how this could happen. Did you discover this in scatter plot (or some other projection)? I was able to replicate this on master. The problem is in scatter plot -- if you disconnect and reconnect it, the colors are OK. I reported this in #4391.

Distance Map does not show selection in an obvious way. Selections are now way too similar to the palette, so they are practically invisible (especially when you use a reddish color palette).

Suggestions? Should Distance map (and similar widgets) be aware of palettes and pick different colors for showing selections? Palette classes could include information about which color is "distinguishable" from those in the palette. Should we do this?

Don't honestly know what to check in Paint Data, Calibration Plot, Lift Curve and ROC Analysis. Or Venn Diagram.

Just that they work. They use new palettes now, but just for getting a list of colors. Not much to go wrong there.

@janezd janezd force-pushed the continuous-palettes branch 4 times, most recently from 5ba56e7 to 5c17c85 Compare January 31, 2020 21:27
@janezd janezd changed the title [RFC] [WIP] Reorganize continuous palettes [ENH] Reorganize continuous palettes Jan 31, 2020
@janezd
Copy link
Contributor Author

janezd commented Feb 1, 2020

I fixed problems found by @ajdapretnar, except for choosing a better color for marking selections, which we have to discuss.

@ajdapretnar, I updated documentation, except images. I'm attaching a colorful screenshot of a widget. I haven't prepared screenshot with workflow and widgets (because shadows and background...). @ajdapretnar, could you? For numeric variables, consider "rainbow". If you like it -- if you don't, don't.

Screenshot 2020-02-01 at 18 15 26

@ajdapretnar
Copy link
Contributor

As for selections, we can address this in another PR I suppose.
I will prepare the documentation. With rainbow, of course, which is one of my favorites.

@ajdapretnar
Copy link
Contributor

I have pushed two commits to your branch. They add the documentation and a sorry attempt at tricking the build script to ignore explicitly unindexed images. I do not know anything about bash, so... 🤷‍♀ It should probably work, but who knows.

@lanzagar lanzagar merged commit f96e566 into biolab:master Feb 7, 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.

Gradient Color Picker
3 participants