Skip to content
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

Change hist behavior when normed and stacked to something more sensible #1833

Merged
merged 4 commits into from
May 9, 2013

Conversation

neggert
Copy link
Contributor

@neggert neggert commented Mar 17, 2013

This addresses #1745. Previously, when passed stacked=True, normed=True, hist would normalize the histogram for each dataset, then stack them, which is, IMO, not what most users would want. With this PR, hist will first stack the histograms, then normalize them such that the integral of the stacked histograms equals 1.

@pelson
Copy link
Member

pelson commented Mar 18, 2013

LGTM 👍

@mdboom
Copy link
Member

mdboom commented Mar 18, 2013

Just to confirm: Is this a change in the API in 1.2.x? If so, we probably should note that in the changelog. If this is a change to something that only ever existed on master, we shouldn't need to.

@neggert
Copy link
Contributor Author

neggert commented Mar 18, 2013

I think the stacked kwarg was introduced in 1.2.x, so yes, we should note it in the changelog.

@dmcdougall
Copy link
Member

Is master the right thing to target here?

d2 = np.linspace(0, 10, 50)
fig = plt.figure()
ax = fig.add_subplot(111)
ax.hist( (d1, d2), stacked=True, normed=True)
Copy link
Member

Choose a reason for hiding this comment

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

PEP8 : there should be no space inbetween the two parenthesis.

@neggert
Copy link
Contributor Author

neggert commented Mar 23, 2013

This does change the API, albeit an obscure corner of it. I think master is
correct.

On Saturday, March 23, 2013, Damon McDougall wrote:

Is master the right thing to target here?


Reply to this email directly or view it on GitHubhttps://github.com//pull/1833#issuecomment-15332648
.

Sent from Gmail Mobile

@neggert
Copy link
Contributor Author

neggert commented Apr 6, 2013

Rebased and fixed PEP8 issues. Hopefully this is good to go now. Sorry for the long delay.

@neggert
Copy link
Contributor Author

neggert commented May 7, 2013

Just noticed this is still open. Is there anything else that needs to be done?

@mdboom
Copy link
Member

mdboom commented May 8, 2013

That's for the ping. Looks fine to me. Would you mind rebasing so that Travis will run one more time as a sanity check before we merge?

@neggert
Copy link
Contributor Author

neggert commented May 8, 2013

Just rebased. I get some font-related test failures, but those are probably unrelated.

mdboom added a commit that referenced this pull request May 9, 2013
Change hist behavior when normed and stacked to something more sensible
@mdboom mdboom merged commit cda4f0d into matplotlib:master May 9, 2013
@neggert neggert deleted the normedstacked_fix branch May 9, 2013 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants