Skip to content

MNT: more control of colorbar with CountourSet #5056

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

Closed

Conversation

tacaswell
Copy link
Member

In the ColorBar constructor values are extracted from the mappable if
it is a ContourSet. Instead of always overwriting user-input only,
respect user input if given.

This came up while looking into #5055

import matplotlib.pyplot as plt
import numpy as np
import matplotlib.colors as mplc

x, y = np.mgrid[0:1:0.01, 0:1:0.01]
r = np.sqrt(x ** 2 + y ** 2)
z = np.sin(6 * np.pi * r)

norm = mplc.Normalize(0, 0.5)
fig0, ax0 = plt.subplots(1, 1, )
cf0 = ax0.contourf(x, y, z, np.arange(0, .5, .01), norm=norm,
                   extend='both', cmap='viridis')
cbar0 = plt.colorbar(cf0, extend='neither')

now results in

so

Currently on master the extend kwarg to colorbar was silently ignored.

Possibly related to #5034 / #4850

attn @efiring @fmaussion @lkilcher

@efiring
Copy link
Member

efiring commented Sep 13, 2015

Why do you want to allow the user to generate a colorbar that is inconsistent with its mappable? I can see a reason for using setdefault for the extend kwarg (even though I think that overriding it is in general a bad idea, yielding a misleading plot), and I can see a reason for having a warning or error if a kwarg is being overridden, but I don't think that using setdefault to allow the user to destroy the linkage between the colorbar and the mappable, without even a warning, is a good idea.

@fmaussion
Copy link
Contributor

#5055 is a good example for things happening automatically in a "good way", i.e. informing the user "you did something wrong in your plot".

About @efiring's comment, it seems to me that it is easy to generate a colorbar that is inconsistent with its mappable, essentially because of the "extend" keyword having to be set by the user. Neither the normalization nor the colorbar classes will warn the user who sets the norm's min/max bounds that the plotted data is actually out-of-bounds. This makes the clip=True keyword quite useless btw: the following code produces the same misleading plot with or without clipping.

import matplotlib.pyplot as plt
import numpy as np
import matplotlib as mpl

x, y = np.mgrid[0:1:0.01, 0:1:0.01]
r = np.sqrt(x ** 2 + y ** 2)
z = np.sin(6 * np.pi * r)

cmap = mpl.cm.get_cmap('PuBuGn')
norm = mpl.colors.BoundaryNorm([0.1, 0.2, 0.3, 0.4], cmap.N)
plt.pcolormesh(x, y, z, norm=norm, cmap=cmap)
plt.colorbar()

test

@tacaswell tacaswell added this to the proposed next point release (2.1) milestone Oct 8, 2015
@efiring
Copy link
Member

efiring commented Nov 9, 2015

@tacaswell, looking at this again, I still object to silently breaking all of the linkages. It makes no sense whatsoever. For extend, OK, but not for the others.

@tacaswell tacaswell force-pushed the enh_cbar_respect_more_kwargs branch from 8c2049e to a9cad4a Compare January 3, 2016 20:26
@tacaswell
Copy link
Member Author

@efiring I have re-jiggered how this works so it now raises a TypeError instead of silently ignoring user input (which does not allow the user to (via the constructor) break the link between the mappable and the colorbar).

will now raise a `TypeError` rather than silently drop user supplied
input.

If ``mappable`` is a `contouer.ContourSet` then a `TypeError` will be raised
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: "contouer"

@tacaswell tacaswell force-pushed the enh_cbar_respect_more_kwargs branch from b938f82 to cc301b7 Compare January 3, 2016 21:31
"""
# deal with default value of alias_mapping
if alias_mapping is None:
alias_mapping = dict()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just pointing out another way that I sometimes do it:
alias_mapping = alias_mapping or {}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not a huge fan of that due to the many other things that are 'falsey' that will trigger the default behavior when (maybe?) they should not. ex, '' should not work as the alias_mapping

@tacaswell tacaswell mentioned this pull request Jan 6, 2016
@tacaswell
Copy link
Member Author

Should I pick this PR apart so we can backport normalize_kwargs to 2.0 and maybe 1.5.x?

In the `ColorBar` constructor values are extracted from the mappable if
it is a ContourSet.  Instead of always overwriting user-input only,
respect user input if given.

If the user passes in kwargs which can be extracted from a ContourSet
raise TypeError instead of silently ignoring user input.
@tacaswell tacaswell force-pushed the enh_cbar_respect_more_kwargs branch from 6127cc6 to 32aaee4 Compare May 13, 2016 01:12
@tacaswell tacaswell modified the milestones: 2.1 (next point release), 2.2 (next next feature release) Sep 24, 2017
@jklymak
Copy link
Member

jklymak commented Aug 17, 2018

This seems useful (w/o spending too much time w/ it). If @tacaswell doesn't have time, maybe I'll take it over, or someone else can try and move it through...

@anntzer
Copy link
Contributor

anntzer commented Jan 3, 2020

Superseded by #16051.

@anntzer anntzer closed this Jan 3, 2020
@tacaswell tacaswell deleted the enh_cbar_respect_more_kwargs branch January 4, 2020 01:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants