Skip to content

Fix bug when stacking histograms with non-integer weights - #1372

Closed
neggert wants to merge 10 commits into
matplotlib:masterfrom
neggert:stacked_hist_fix
Closed

neggert wants to merge 10 commits into
matplotlib:masterfrom
neggert:stacked_hist_fix

Conversation

@neggert

@neggert neggert commented Oct 12, 2012

Copy link
Copy Markdown
Contributor

Fixes #1370.

Also run boilerplate.py to update pyplot.py with correct signature for hist.

@WeatherGod

Copy link
Copy Markdown
Member

Can we have a test so that this doesn't crop back up.

Also, I have to wonder why the original code explicitly cast the zeros as integers? Maybe there was a reason for it?

@neggert

neggert commented Oct 15, 2012

Copy link
Copy Markdown
Contributor Author

I'll work on a test for this.

I noticed that when running the existing tests, I get this warning

/usr/local/lib/python2.7/dist-packages/matplotlib/axes.py:8080: DeprecationWarning: Implicitly casting between incompatible kinds. 
In a future numpy release, this will raise an error. Use casting="unsafe" if this is intentional.     
  m += mlast 

It seems like np.histogram can return an array of either floats or ints depending on what it's given as input. We probably want to get rid of this warning, so I'll change this to grab the type from the np.histogram bins.

@dmcdougall

Copy link
Copy Markdown
Member

+1.

I milestoned this for 1.2. Regarding the discussion on the mailing list, 1.2 should be reserved only for critical bugs. Does this qualify as critical?

Improve "Report a problem" to "Getting help"
@efiring

efiring commented Oct 15, 2012

Copy link
Copy Markdown
Member

It looks like something it would be very nice to include. I would prefer seeing it rebased against v1.2.x and merged there directly rather than cherry-picked.

@neggert

neggert commented Oct 15, 2012

Copy link
Copy Markdown
Contributor Author

Alright, rebased onto 1.2.x. That was a learning experience in git.

Was that the proper way to rebase it? I notice it seems to have grabbed a bunch of 1.2.x changes and included them in the PR.

@neggert

neggert commented Oct 15, 2012

Copy link
Copy Markdown
Contributor Author

Nevermind, figured it out. See #1406.

@efiring efiring closed this Oct 16, 2012
@efiring

efiring commented Oct 16, 2012

Copy link
Copy Markdown
Member

@neggert, Thanks very much for the rebase, which I committed, as well as the original PR.

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.

Bug in #847 (stacked histograms)

5 participants