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

Bug in multiple step horizontal histograms #2830

Closed
ycopin opened this issue Feb 21, 2014 · 8 comments
Closed

Bug in multiple step horizontal histograms #2830

ycopin opened this issue Feb 21, 2014 · 8 comments

Comments

@ycopin
Copy link

ycopin commented Feb 21, 2014

Title says it all (using matplotlib 1.3.1):

hist([randn(100),randn(100)],orientation='horizontal', histtype='step')

bug

The 2nd histo is just going nuts. This works fine for traditional 'vertical' orientation and/or non-stepped histos.

@ycopin
Copy link
Author

ycopin commented Feb 21, 2014

I think I found the bug. In axes.py, function hist (line 8467), the following piece of code

                if orientation == 'horizontal':
                    x, y = y, x

                xvals.append(x.copy())
                yvals.append(y.copy())

is wrong: the x-y exchange on the 1st histo will mess up the following ones. It should be replaced by:

                if orientation == 'horizontal':
                    xvals.append(y.copy())
                    yvals.append(x.copy())
                else:
                    xvals.append(x.copy())
                    yvals.append(y.copy())

bug2

@tacaswell tacaswell added this to the v1.3.x blocker milestone Feb 21, 2014
@tacaswell
Copy link
Member

Can you put in a PR with this fix? It would also be great if you could add a test.

@ycopin
Copy link
Author

ycopin commented Feb 21, 2014

I'm sorry I'm not at ease w/ PR and tests (see issue #841). I was hopping a more instructed developer would just incorporate my simple bugfix... :-/

@tacaswell
Copy link
Member

@neggert, can you take a look at this?

@neggert
Copy link
Contributor

neggert commented Feb 22, 2014

Yeah, I can put this in tomorrow. This should go in the v1.3.x branch?

@tacaswell
Copy link
Member

Yes to 1.3.x.

We are starting to triage for the 1.4 release and it is unclear if 1.3.2
will ever happen, but off it is no extra work might as well put it on 1.3.x.
On Feb 21, 2014 8:38 PM, "Nic Eggert" notifications@github.com wrote:

Yeah, I can put this in tomorrow. This should go in the v1.3.x branch?


Reply to this email directly or view it on GitHubhttps://github.com//issues/2830#issuecomment-35790882
.

@tacaswell
Copy link
Member

@neggert Can I pester you about this? If not, I can deal with it, but you are much more familiar with the histogram code.

tacaswell added a commit that referenced this issue Feb 26, 2014
Fix bug in horizontal step histograms (#2830)
@ycopin
Copy link
Author

ycopin commented Feb 27, 2014

Thanks for the PR: I now have a better view of what's called a test (I didn't know you could just compare to a reference image).

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

No branches or pull requests

3 participants