When creating a LogNorm imshow plot with more than 9 orders of magnitude of dynamic range the automatically generated colorbar from plt.colorbar() has no automatically generated tick marks or labels.
@jklymak what about the minor ticks? does #9903 change that as well? If so we'll need to fix that in yt for the next matplotlib release after #9903 gets merged. This behavior also changed with matplotlib 2.0 - we have some matplotlib version-specific hacks in yt already for this:
But what we really want is for minor ticks to be drawn on the colorbar as they would be on a normal axis, that is, they should be shorter than the major ticks.
@ngoldbaum You don't really want minor ticks when you get more than two or more decades between major ticks. They will look very funny. Thats why you were getting a blank before.
We could probably tweak the auto tick location to let there be more major ticks, but...
If the minor ticks get drawn in when there's less dynamic range I'm fine with that. It does seem like a nice cleanup of colorbar handling - if that can get more sane I'm ok with there being some changes in behavior.
In general yt should just do whatever matplotlib does here and not worry too much about whether there are minor in where the tick labels are differences on old versions.
OK, great. I looked at the LogLocator code. It definitely drops the minor ticks if there is more than a decade between major ticks. I think this is perfectly reasonable. I also think its relatively straightforward for you to hard-code the minor ticks if you think you really want them. Or write a custom version of LogLocator if you want some automation. Happy to help w/ that if you guys decide you need that. (you basically just remove a check for the number of strides to be 1)
Nah, i don't think it makes any sense to draw log minor ticks when there's more than one order of magnitude between major ticks.
#9903 looks great! It's bugged me in the past that there are all these subtle differences between the norm locators so making things more uniform is very helpful for people building plotting code on top of matplotlib. It'll also be nice that the number of ticks scales naturally with the size of the colorbar.
#9903seems to work well, but it does some violence to the way that colorbars are made. (long story is that instead of the pcolor object's yaxis going from 0-1, it goes from vmin to vmax, just like any other axis. But it only does this for the default Norm and LogNorm) So it may not make it into Master. If not, then we should revisit this case. I'm sure we can make it work OK w/ the existing code.
No worries if the answer is that it would be a pain to fix or no one wants to, I've only seen a few reports about this behavior from yt users and I don't think it's a common issue.
I’ve not looked at this case in isolation. I imagine a PR would be welcome, but I’d argue #9903 obviates the need, and would hence be a bit of a waste of time. But nothing got in for 2.2 so if someone could do something quick for 2.2 that might be useful.
Activity
jklymak commented on Dec 20, 2017
#9903 fixes this:
ngoldbaum commented on Dec 20, 2017
@jklymak what about the minor ticks? does #9903 change that as well? If so we'll need to fix that in yt for the next matplotlib release after #9903 gets merged. This behavior also changed with matplotlib 2.0 - we have some matplotlib version-specific hacks in yt already for this:
https://github.com/yt-project/yt/blob/b83691665f6574d7bf1b01c7bd69088c32e1d6a9/yt/visualization/plot_window.py#L925
It would be nice if you could make it so that the minorticks get drawn just like the current verison.
efiring commented on Dec 20, 2017
But what we really want is for minor ticks to be drawn on the colorbar as they would be on a normal axis, that is, they should be shorter than the major ticks.
jklymak commented on Dec 20, 2017
Like this (from #9903)?
Edit: Just to be clear this is with 1e9 instead of 2e9
jklymak commented on Dec 20, 2017
@ngoldbaum You don't really want minor ticks when you get more than two or more decades between major ticks. They will look very funny. Thats why you were getting a blank before.
We could probably tweak the auto tick location to let there be more major ticks, but...
ngoldbaum commented on Dec 20, 2017
If the minor ticks get drawn in when there's less dynamic range I'm fine with that. It does seem like a nice cleanup of colorbar handling - if that can get more sane I'm ok with there being some changes in behavior.
In general yt should just do whatever matplotlib does here and not worry too much about whether there are minor in where the tick labels are differences on old versions.
jklymak commented on Dec 20, 2017
OK, great. I looked at the LogLocator code. It definitely drops the minor ticks if there is more than a decade between major ticks. I think this is perfectly reasonable. I also think its relatively straightforward for you to hard-code the minor ticks if you think you really want them. Or write a custom version of LogLocator if you want some automation. Happy to help w/ that if you guys decide you need that. (you basically just remove a check for the number of strides to be 1)
ngoldbaum commented on Dec 20, 2017
Nah, i don't think it makes any sense to draw log minor ticks when there's more than one order of magnitude between major ticks.
#9903 looks great! It's bugged me in the past that there are all these subtle differences between the norm locators so making things more uniform is very helpful for people building plotting code on top of matplotlib. It'll also be nice that the number of ticks scales naturally with the size of the colorbar.
jklymak commented on Dec 20, 2017
#9903 seems to work well, but it does some violence to the way that colorbars are made. (long story is that instead of the pcolor object's yaxis going from 0-1, it goes from vmin to vmax, just like any other axis. But it only does this for the default Norm and LogNorm) So it may not make it into Master. If not, then we should revisit this case. I'm sure we can make it work OK w/ the existing code.
ngoldbaum commented on Feb 20, 2018
@jklymak was there a fix for this in the current colorbar implementation for 2.2?
jklymak commented on Feb 20, 2018
No #9903 is not merged yet. It needs a bit of a rebase, but is reviewable as is.
ngoldbaum commented on Feb 20, 2018
Sorry, i mean a fix for this issue separate from #9903 (as you suggested in your comment from December), since it looks like #9903 won't land in 2.2.
ngoldbaum commented on Feb 20, 2018
No worries if the answer is that it would be a pain to fix or no one wants to, I've only seen a few reports about this behavior from yt users and I don't think it's a common issue.
jklymak commented on Feb 20, 2018
I’ve not looked at this case in isolation. I imagine a PR would be welcome, but I’d argue #9903 obviates the need, and would hence be a bit of a waste of time. But nothing got in for 2.2 so if someone could do something quick for 2.2 that might be useful.
ngoldbaum commented on Aug 12, 2018
@jklymak now that #9903 is merged I guess this is fixed? I haven't tested with 3.0rc1 although I guess I could tomorrow when I test yt with the rc.
ngoldbaum commented on Aug 13, 2018
This is indeed fixed, I'm closing.
jklymak commented on Aug 13, 2018
Phew! I was sweating for a while.... 😉 Thanks so much for testing the RC!