Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork8.1k
Add log option to Axes.hist2d#1061
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
lib/matplotlib/axes.py Outdated
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.
Don't you mean 'np.log1p(h)' here? Plus, let's have this kwarg better handled. First, let us stick with the convention of setting the default to None. This will allow for future revisions to treat that to mean "the default behavior", which usually means to grab from the rc-params. Second, let us take the opportunity now to choose either True/False or string values as valid inputs to the kwarg -- not both. This has caused so much grief elsewhere. If you can imagine that other log scales (or even more generally, other z scales) would be desired, then let's go with strings. Otherwise, stick with booleans.
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.
Bah, yes, you are right. This is why one shouldn't write code in Github's editor.
In fact, the patch isn't even consistent in that it documents three values for the log kwarg, yet only implements only one. I was originally thinking of allowing the user to choose to use eithernp.log ornp.log1p. Given we are talking about histograms here, it seems thatlog1p or similar is the only reasonable approach as we expect bins with zero counts. Consequently, I'd probably assume just make the option boolean and always uselog1p (this would need to be made clear in the documentation, of course). Anyone have any better ideas?
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.
hist2d takes advantage of the fact that when an element in array is set toNone the bin representing the value is white out which looks nice. From my little test, log1p dies onNone. You may want to take care ofNone properly. Try this:
x=randn(100)y=randn(100)hist2d(x,y,cmin=1)
WeatherGod commentedAug 9, 2012
Interesting feature, and I definitely support the idea because the user has no other way to scale the histogram before display. More work will be needed on this PR, however, to get it up to snuff.
Also desirable (let us know if you need help with either of these):
|
bgamari commentedAug 9, 2012
Sure, I completely understand that more work is necessary. I largely opened to the request to see if people thought this would be a reasonable idea. I'll try to put some time in tonight to bring this into a mergeable form. |
bgamari commentedAug 15, 2012
Is there any way to run |
WeatherGod commentedAug 15, 2012
No, it.assumes that whatever install of mpl is the one you are using to |
piti118 commentedAug 15, 2012
If log is done this way, will the colorbar label come out right?. I mean, will the label show the value or log of value? |
WeatherGod commentedAug 16, 2012
Ooh, good point, I hadn't thought of that. It would show, by default the |
bgamari commentedAug 21, 2012
I believe I touched on most of the changes mentioned above in this updated series. Things have changed quite a bit,
How does it look to others? |
lib/matplotlib/axes.py Outdated
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.
Doesn't this need to be fixed with the updated code? Since it is no longer counts+1, right?
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.
Oops. Yes, of course.
bgamari commentedAug 22, 2012
Here is a quick patch adding a note pointing out how to use a logarithmic color scale. It might also be nice to demonstrate how to do this in |
bgamari commentedAug 26, 2012
Any opinion on the above? It might be nice to see a patch along these lines go in to 1.2. |
lib/matplotlib/axes.py Outdated
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.
Do
:class:`colors.LogNorm`instead. Note the backticks.
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.
Done.
bgamari commentedAug 28, 2012
Any thoughts concerning adding a logarithmic histogram to |
WeatherGod commentedAug 28, 2012
That would be a great idea. |
WeatherGod commentedAug 28, 2012
Note, that I think it would be best to have it as a separate example |
bgamari commentedAug 28, 2012
Alright, that is fine. I'll submit a patch with a new example shortly but do you suppose 3f2cdaf could be merged in the meantime? |
bgamari commentedAug 28, 2012
How does this look as an example? |
lib/matplotlib/axes.py Outdated
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.
single colon, not double colon
bgamari commentedAug 30, 2012
I think this is ready for merging. Any other requests? |
lib/matplotlib/axes.py Outdated
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.
Sorry I didn't notice this before, but we need backticks around "pcolorfast".
bgamari commentedAug 30, 2012
Good catch. Fixed. |
travisbot commentedAug 30, 2012
bgamari commentedAug 30, 2012
Not entirely sure what@travisbot is upset about but I don't see anything that could be from my (quite trivial) work. |
WeatherGod commentedAug 31, 2012
Don't fret. We are still working on configuring our automated testing |
WeatherGod commentedSep 4, 2012
Good work@bgamari, I will merge. |
We use np.log1p since one would usually expect to find bins with 0 counts.