Adding density hist2d #12563
Adding density hist2d #12563
Conversation
lib/matplotlib/axes/_axes.py
Outdated
| "simultaneously. Please only use 'density', " | ||
| "since 'normed' is deprecated.") | ||
| if normed is not None: | ||
| cbook.warn_deprecated("2.1", name="'normed'", obj_type="kwarg", |
There was a problem hiding this comment.
You cannot deprecate for "2.1". The deprecation has to be always for the next point release, i.e. currently "3.1". removal can be omitted. That's two minor point releases by default.
lib/matplotlib/axes/_axes.py
Outdated
| normed : bool, optional, default: False | ||
| Normalize histogram. | ||
| density : boolean, optional | ||
| If False, the default, returns the number of samples in each bin. |
There was a problem hiding this comment.
"returns" is a bit misleading. I would use "plots and returns".
b2daa26 to
d063002
Compare
|
@timhoffm Thanks for the comments. I was getting hard time rebasing. Last time I rebased my branch, I got other people commits on my branch and I just rebase this branch now and it looks fine after following this. So |
| If False, the default, plots and returns the number of samples | ||
| in each bin. If True, returns the probability *density* | ||
| function at the bin, ``bin_count / sample_count / bin_area``. | ||
| Default is ``None`` for both *normed* and *density*. If either is |
There was a problem hiding this comment.
Can we keep the discussion of deprecated normed kwarg in the description of that kwarg?
| cbook.warn_deprecated("3.1", name="'normed'", obj_type="kwarg", | ||
| alternative="'density") | ||
|
|
||
| normed = bool(density) or bool(normed) | ||
| h, xedges, yedges = np.histogram2d(x, y, bins=bins, range=range, |
There was a problem hiding this comment.
I don't think we should call the argument normed if we are going to change to density later.
This could also use a comment in here about how we will need to switch to density when np1.15 becomes our minimal version. Otherwise if I was looking at this code I'd have no idea why you are deprecating the name normed when that's what numpy is using. Indeed reviewing this I had to see @anntzer 's comment: #11070 (comment) to understand why you were doing this. Should probably be in the PR description as well to save folks back referencing.
There was a problem hiding this comment.
I'm fine with the change proposed here , but only because numpy is moving to using density as well (in np1.15). So the PR should be clear why we are doing this.
There was a problem hiding this comment.
So you still have normed = bool(density) or bool(normed), I think you should be populating and using density not normed.
I think there should be a comment in the code here for future reference to explain this.
There was a problem hiding this comment.
I misunderstood about your comment on density. Let me fix it right away.
653be4e to
14a74ac
Compare
lib/matplotlib/axes/_axes.py
Outdated
| @@ -6869,8 +6878,20 @@ def hist2d(self, x, y, bins=10, range=None, normed=False, weights=None, | |||
| keyword argument. Likewise, power-law normalization (similar | |||
| in effect to gamma correction) can be accomplished with | |||
| `.colors.PowerNorm`. | |||
| - Numpy 1.15 introduced a 'density' kwarg to ``hist2d``. Even though | |||
| Numpy 1.15 isn't required, 'density' kwarg is introduced | |||
| and passed as 'normed' to ``hist2d``. | |||
There was a problem hiding this comment.
@jklymak Do you want me to place this explanation somewhere else? Thanks.
There was a problem hiding this comment.
This probably doesn’t hurt I might have put it in the normed kwarg description
efb6cb2 to
ab1765d
Compare
|
@thoo please ping me when this is ready and ask for a re-review! |
|
@jklymak Can you re-review this PR? |
|
This was covered by #13128. |
Close #11070