Repository navigation
MPL 3 + Colorbar + PowerNorm bug #13228
Description
Bug report
Bug summary
In MPL v3, when using Colorbar with a linear norm, then changing norm to PowerNorm, breaks the Colorbar under some circumstances. Worked fine in MPL v1 and v2.
Code for reproduction
Minimum necessary test case:
fig, ax = plt.subplots()
data = np.clip(randn(250, 250)*10, 0, 10)
cax = ax.imshow(data)
cbar = fig.colorbar(cax)
cax.norm = mpl.colors.PowerNorm(gamma=2, vmin=0, vmax=10)
cbar.set_norm(cax.norm)
cbar.update_normal(cax)
plt.show()On matplotlib 3.0.2 the (bug?) result is:
On matplotlib 2.2.3 the (expected) result is:
Result when adding LogNorm to ColorbarBase._use_auto_colorbar_locator (see discussion below) on matplotlib 3.0.2
Discussion
Few points:
- If the image's norm is set prior to
fig.colorbar, then the expected result is achieved on 3.0.2 as well. - If vmin/vmax are not set, the result seems correct and matches between 3.0.2 and 2.2.3
cbar.update_bruteforceinstead of update_normal does not fix the issue
I looked into the colorbar code, and can see the following:
In ColorbarBase.update_ticks(), if the auto-colorbar is first used (i.e. first branch), then setting PowerNorm calls the second branch (i.e. what it calls the fixed locator branch) once Colorbar.update_normal runs. That exact sequence triggers the issue.
If things are forced to the first branch of ColorbarBase.update_ticks() (e.g. via adding to PowerNorm to ColorbarBase._use_auto_colorbar_locator() ), then a similar result to what one gets from MPL 3.0.2 LogNorm is achieved. If things are forced to the second branch, then the result from MPL 2.2.3 is achieved. However, when first one branch is hit and then PowerNorm hits the second branch, this triggers the issue.
Relevant code from ColorbarBase,
def _use_auto_colorbar_locator(self):
return (self.boundaries is None
and self.values is None
and ((type(self.norm) == colors.Normalize)
or (type(self.norm) == colors.LogNorm)))
def update_ticks():
# ...
if self._use_auto_colorbar_locator():
_log.debug('Using auto colorbar locator on colorbar')
_log.debug('locator: %r', locator)
long_axis.set_major_locator(locator)
long_axis.set_major_formatter(formatter)
else:
_log.debug('Using fixed locator on colorbar')
ticks, ticklabels, offset_string = self._ticker(locator, formatter)
long_axis.set_ticks(ticks)
long_axis.set_ticklabels(ticklabels)
long_axis.get_major_formatter().set_offset_string(offset_string)Expected outcome
Either:
- functionality of MPL 2.2.3
- functionality of adding PowerNorm to
ColorbarBase._use_auto_colorbar_locator.
Activity
jklymak commentedon Jan 19, 2019
Not fully parsing this. But if PowerNorm is added to the list in _use_auto_color_locator does everything work?
Note colorbar handling was changed 3.0 for as many cases as possible but it’s possible that it doesn’t fit this pattern of calling things.
QuLogic commentedon Jan 19, 2019
This looks something like #12682.
jklymak commentedon Jan 19, 2019
cbar.set_norm(cax.norm). If you change the mappable, we should be able to deal with the new norm on the mappable.update_normalandupdate_bruteforceare definitely messed up.My test code uses
LogNorm, just to make it clear that the issue is notPowerNorm.LevN0 commentedon Jan 20, 2019
@jklymak - Yes, the specific issue I reported with PowerNorm does appear to work if added to _use_auto_color_locator, though I did limited experimentation. See 3rd image in above report for what that particular result looks like if that change was made. I am not exactly sure why hitting the second branch after running the first produces such a strange outcome though, I could not dig down that far. Some solution to the issue would be great, because this has worked since at least ~v1.0.
Regarding LogNorm, I am not actually seeing much change in your test code/plots compared to behavior in 2.2.3. Some things are clearly inconsistent, but at least all of them can be fixed with a couple lines of code. The issue at the top is different between 2.2.3 and 3.0.2, and the only way I can see to fix them is to revert the action of
long_axis.set_major_locator(locator)andlong_axis.set_major_formatter(formatter), and this is new behavior. Actually there was an additional thing introduced in that LogNorm sets log axis for colorbar as of MPL v3, but update_normal does not revert to linear if the norm is changed - this is kind of unfortunate, though at least update_bruteforce does work in that case. The PowerNorm issue is not only new, but also not resolved with either update_normal or update_bruteforce.@QuLogic - Agreed on similarity.
jklymak commentedon Jan 20, 2019
OK, Thanks. sure we can make this work - I just had no idea folks would set the norm post-facto so that never got tested. Clearly our testing is inadequate for this feature. But if the above is a good enough test, then I think we can figure out how to make
update_normalwork.Maybe someone should comment on what theidea behaviour should be here. To me:
should just work, and we shouldn't have to call anything like
update_normalorupdate_bruteforce.ping @efiring because I think he will know the history here...
LevN0 commentedon Jan 20, 2019
Sounds good to me!
To explain a bit of reasoning here, the application in question allows user to change the norm. So it comes up as linear by default, but lets them change norm and min/max of norm. Similar to functionality of e.g. SAO DS9.
jklymak commentedon Jan 20, 2019
Are you able to download PRs and test from the dev version? I think I have a fix, but it'd be good to know it works for you
LevN0 commentedon Jan 20, 2019
Thank you for working on this! This partially resolves the issue, however here is another method to trigger it:
This is identical to the original test case, except for the
cbar.set_ticksaddition. The same issue occurs if ticks are added directly to fig.colorbar call. The ticks manually set in the example are identical to what MPL v3 comes up with on its own if no ticks are specified.jklymak commentedon Jan 20, 2019
OK, I'd propose that changing the norm resets the locator and formatter to whatever the default is for that norm, so if you want to specify ticks, you'd need to do so again after changing the norm. So the following
gives
LevN0 commentedon Jan 20, 2019
I noticed that,
Adding the two new lines, that reset the axis prior to
self._tickerseems to cure the originally reported issue. Internallyself._tickercalls create_dummy_axis on both locator and formatter, so perhaps it is not supposed to have any other axis prior to that? I do not have sufficient understanding to know what ill effects it might have though.jklymak commentedon Jan 20, 2019
@LevN0 Can you check vs #13234 and post any code that doesn't work there? I'm not sure how you are getting into the
else:part of that loop.LevN0 commentedon Jan 20, 2019
@jklymak Apologies, did not realize there had been a further change in the PR since it was first referenced.
Indeed, the linear -> PowerNorm -> linear works here. Thank you!
A couple possibly related questions. Not sure if another issue would be more appropriate for the first. Here is a test case,
The result maintains a log axis, and spits out a bunch of warnings.
I believe the issue is that LogNorm sets axis to log, and nothing in update_normal or elsewhere adjusts it back on norm change. Perhaps there should be an else in
ColorbarBase.config_axis?The other part is that the new scheme for LogNorm to use log-axis seems to effectively prohibit arbitrary ticks, since they do not show up unless they fall on a major tick mark. Is there any way around this? An option to maintain linear axes for LogNorm as had been the case in v1 and v2 might have been nice, but this maybe asking too much.
jklymak commentedon Jan 20, 2019
OK, you shouldn't set the mappables norm by hand, but should use
set_norminstead. I agree we shouldn't exposecax.norm, but that will be hard to deprecate.But agreed this had some issues, so see update.
LevN0 commentedon Jan 20, 2019
All the previously reported issues seem gone. A change seems to have broken (it is ignored) the
formatattribute of Colorbar, which worked in MPL 3.0.2. E.g., see:I am (very positively) surprised by the high level of effort you are giving to this issue, thanks!
jklymak commentedon Jan 21, 2019
Well, I broke it, so I have to fix it.
We are running into the issue that the user can set and access
cbar.locator/formatterdirectly, but that we want it to magically reset itself when the norm changes. I think we can still make this all work, but its turning out to not be trivial.efiring commentedon Jan 21, 2019
Would it help to turn some attributes into properties?
jklymak commentedon Jan 21, 2019
I think so but o have one other thing to try first.
jklymak commentedon Jan 21, 2019
I guess the real question is if we want user set locators and formatters to survive a norm change or not. This PR says not. If we want yes, but still do something intelligent if it’s never been set then we prob need properties.
jklymak commentedon Jan 21, 2019
OK, #13234 has addressed all the issues I think we've identified so far, and added tests for them all...
Ended up not needing properties.
@LevN0 - care to see how it is working now?
LevN0 commentedon Jan 21, 2019
All the previously reported issues seem fixed. I could not find any issues with this version, though I am sure my tests are super incomplete.
A question: why is
Colorbar.minorticks_offdisabled for log-axis? I understand minor ticks being turned on by default for LogNorm, but why is the user disallowed from turning them off? If the user adjusts the major ticks via set_ticks and does not have them at powers of 10, minor ticks which seem to locate for powers of 10, make the colorbar more confusing in my opinion.jklymak commentedon Jan 21, 2019
OK< great. I'll mark it as not WIP
The minor ticks thing is a nuisance as well, but can you open a new Issue for that? I think it can be dealt with in another PR. The current PR is long enough as it is..
LevN0 commentedon Jan 21, 2019
Will do, thanks for the work on this one!
jklymak commentedon Feb 3, 2019
Re-milestonig 3.1. This is a bug, but the fix is too intrusive to be for a big-release (and 3.1 is on its way soonish anyway).