-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Fix figure.colorbar() with axes keywords #10000
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
Conversation
Congratulations on getting issue #10000 :-) |
Thanks! |
Can we have a test that checks these kwargs are stripped so no one monkeys w/ this down the road? |
880fe2c
to
9ef39d6
Compare
@jklymak I've added a test. It just checks that the arguments are accepted and do not raise an exception. |
|
||
# need to remove kws that cannot be passed to Colorbar | ||
NON_COLORBAR_KEYS = ['fraction', 'pad', 'shrink', 'aspect', 'anchor', | ||
'panchor'] |
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.
Does the remove of these kwargs mean the docstring was wrong/needs changing now?
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 don't think so. The notes section in the docs says says:
Additional keyword arguments are of two kinds:
- axes properties
- colorbar properties
We receive a mixture, which AFAICS is intended. But we have to remove the kwargs, that Colorbar
does not understand.
Fixes #8493.
The problem was that
Figure.colorbar()
acceptsAxes
as well asColorbar
keywords. However they are all passed on to theColorbar
constructor. This has an explicit signature of supported keywords and complains on theAxes
keywords 'anchor' and 'panchor'.The fix is to filter out
Axes
keywords that are not allowed inColorbar
. Note: 'orientation' has to be passed to both.