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

BUG/API : fix color validation #4193

Merged
merged 2 commits into from Apr 20, 2015
Merged

Conversation

tacaswell
Copy link
Member

Possible fix for #4192.

This adds a new validation (validate_color_or_None) method for color
which allows None and restores validate_color to fail on None.

This will allow selected color rcparams to be None (not 'None')
which the library should interpret as "don't use this rcparam".

Not 100% that this fixes all of the problems.

@efiring
Copy link
Member

efiring commented Mar 5, 2015

The original idea seems to have been that None could be used in the matplotlibrc file to mean the object None; your patch does not permit that. When the matplotlibrc file is read, everything starts out as a string, and that is the way it is passed to the validator. It is not the object None, so it will be passed on to the color validator and converted to 'None', which is not what was intended. This will happen if the user uncomments the line in the matplotlibrc template. Very confusing. The attempt to use None here is a mistake; or maybe the mistake, long ago, was using 'none' to mean no color.

Instead of None, one could use "from_axes"; then validate_color_or_from_axes(); and handle from_axes in the legend code.

@efiring
Copy link
Member

efiring commented Mar 5, 2015

An alternative to "from_axes" would be "inherited".

@tacaswell
Copy link
Member Author

Ahhh, I understand my confusion now.

On Wed, Mar 4, 2015, 21:26 Eric Firing notifications@github.com wrote:

An alternative to "from_axes" would be "inherited".


Reply to this email directly or view it on GitHub
#4193 (comment)
.

@tacaswell tacaswell modified the milestones: Color overhaul, next point release Mar 10, 2015
@tacaswell tacaswell force-pushed the fix_color_rc branch 2 times, most recently from 636003a to 174499e Compare March 14, 2015 01:51
Possible fix for matplotlib#4192.

This adds a new validation (validate_color_or_None) method for color
which allows None and restores `validate_color` to fail on None.

This will allow selected color rcparams to be `None` (not `'None'`)
which the library should interpret as "don't use this rcparam".
Attempt to implement 'inherited' rcparams, that is rcparams
that are only used if they are non-default.

Re-introduces feature introduced in matplotlib#3792
/92e608d655f1fa667fdf5bc3e99f950eb08f7c42 and reverted in
c90469b
@tacaswell
Copy link
Member Author

@efiring Try number 3 at fixing this.

@tacaswell
Copy link
Member Author

attn @mrkrd

@tacaswell
Copy link
Member Author

@efiring up or down on this?

@efiring
Copy link
Member

efiring commented Apr 19, 2015

I like it.

WeatherGod added a commit that referenced this pull request Apr 20, 2015
BUG/API : fix color validation
@WeatherGod WeatherGod merged commit 2b9c228 into matplotlib:master Apr 20, 2015
@tacaswell tacaswell deleted the fix_color_rc branch April 20, 2015 16:00
tacaswell added a commit that referenced this pull request Apr 21, 2015
FIX : rcParams legend.facecolor and edgecolor never being used

cleans up #4193
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.

None yet

3 participants