-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
ENH: Styler.bar
accepts matplotlib colormap
#43662
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
Merged
Merged
Changes from 8 commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
0dc9a65
add matplotlib colors to styler.bar
attack68 9420586
add tests
attack68 61d52a5
mypy fix
attack68 23ae4aa
Merge remote-tracking branch 'upstream/master' into styler_bar_colors
attack68 67b863b
add tests
attack68 babe2cc
update user guide
attack68 051d313
docs
attack68 d395c4d
whats new
attack68 bae65ec
Merge branch 'master' into styler_bar_colors
attack68 a385f75
Merge branch 'master' into styler_bar_colors
attack68 e49957a
refactor args
attack68 74f249e
Merge remote-tracking branch 'upstream/master' into styler_bar_colors
attack68 3ed226a
Merge remote-tracking branch 'upstream/master' into styler_bar_colors
attack68 1f88b0f
Merge remote-tracking branch 'upstream/master' into styler_bar_colors
attack68 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
don't we have a similar utlity in the plotting routines?
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.
had a quick look, this is converting mpl colors to hex codes, which is not the usual direction for plotting.
ignoring the arg validation, though which is specific to the method, this op is just a single loop comprehension:
[mpl_colors.rgb2hex(rgba) for rgba in rgbas]
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.
an alternative is to add a
cmap
arg, which accepts a string or mpl ColorMap, similar tobackground_gradient
andtext_gradient
.Advantage is: i) consistency ii) autoloading a colormap from string rather than user have to create a Colormap instance.
Disadvatange is: having a
color
and acmap
arg which is slightly confusing although can b documented away.?
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.
hmm i do like the consisteny of i), can we kill the
color
arg (may need to deprecate)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.
not really because you need differentiate between a color which is a valid css str e.g. "#ee3ee" or "rgb(10,200,10)" or "salmon" and between a string which is a matplotlib colormap, i.e. "inferno" or "PuBu",
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.
do we have this color, cmap api anywhere else?
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.
The following use
color
:highligh_min/max/betwen/quantile/null
, and these usecmap
:text/background_gradient
. But the use is specific where either a specific color or a color gradient is useful.For
bar
either is useful.Matplot lib also separates the arguments on some functions, e.g.
scatter
, where it has ac
,color
andcmap
argument: https://matplotlib.org/stable/api/_as_gen/matplotlib.axes.Axes.scatter.html?highlight=scatter#matplotlib.axes.Axes.scatterThere 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.
hah, mpl api is not great here