Skip to content

Fix bugs in stacked histograms - #1706

Closed
neggert wants to merge 2 commits into
matplotlib:masterfrom
neggert:hist_stack
Closed

neggert wants to merge 2 commits into
matplotlib:masterfrom
neggert:hist_stack

Conversation

@neggert

@neggert neggert commented Jan 26, 2013

Copy link
Copy Markdown
Contributor

Hi,

This is an attempt to fix both #1679 and #1631, which both came about because of the way that stacked histograms are handled, introduced in #847.

This isn't ready to go in yet, but I'm running into some problems getting it to work correctly, so I thought I'd try to get some more eyes on it.

There are two problems I'm running into:

  1. fill_between isn't actually filling
  2. fill_between seems to be screwing up autosetting the y axis limits.

Here is the image I get when I run test_hist_stacked. The two histograms are now in the proper order, fixing #1679, you can see the two problems I mentioned. The histograms should be filled and the minimum of the y-axis should be 0.

hist_stacked

Could someone a little more familiar with fill_between take a look at this?

Comment thread lib/matplotlib/axes.py

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Since the switch to fill_between, we're dealing with a PolyCollection rather than a list of patches. Thus, some changes here. I also put the patches from the bar histtype into a PatchCollection, so they can be treated using the same interface.

@dmcdougall

Copy link
Copy Markdown
Member

This currently targets master. Should it instead target v1.2.x if its intent is to fix a bug?

@neggert

neggert commented Jan 26, 2013

Copy link
Copy Markdown
Contributor Author

I suppose you're correct. Let's get it working first, then I'll switch it over.

@WeatherGod

Copy link
Copy Markdown
Member

If this changes the output type of the call to PolyCollection from a list
of patches, we can't make this go to v1.2.x branch since it would be a
change in API.

@neggert

neggert commented Jan 28, 2013

Copy link
Copy Markdown
Contributor Author

I think we can keep the API the same by just returning patches.get_paths(). I'm not particularly attached to using the collections, that's just what fill_between returns. If we can't get fill_between to behave, this doesn't matter anyway.

@dmcdougall

Copy link
Copy Markdown
Member

I think we can keep the API the same by just returning patches.get_paths().

Actually, not only the type of the returned value needs to be the same, but the order in which they appear in the returned iterable needs to be unchanged as well.

@neggert

neggert commented Jan 28, 2013

Copy link
Copy Markdown
Contributor Author

Let's cross that bridge when we come to it. Right now, this isn't even working properly.

Comment thread lib/matplotlib/axes.py

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

PEP8 nitpick: this line is under-indented: it should be align with the opening ( of the previous line

@neggert

neggert commented Jan 30, 2013

Copy link
Copy Markdown
Contributor Author

Closed in favor of #1723.

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.

4 participants