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

Add new plot style: stackplot #819

Closed
wants to merge 13 commits into from

Conversation

dmcdougall
Copy link
Member

This addresses issue #359.

There is also an example so you can try out the pull request. Sample output from this code is provided below:

stackplot

Feedback is greatly appreciated, as always.

@dmcdougall
Copy link
Member Author

Hooray, only two commits this time!

@piti118
Copy link
Contributor

piti118 commented Apr 11, 2012

+1

Parameters
----------
*x* : 1d array of dimension N
*y* : 2d array of dimension MxN
Copy link
Member

Choose a reason for hiding this comment

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

We should probably clarify here what's in the comment below: that the data is not "pre-stacked".

It might also be nice for y to be either a 2d array of MxN, or a variable number of y arguments (which would be stacked implicitly here).

@mdboom
Copy link
Member

mdboom commented Apr 13, 2012

This is great! Barring my minor comments, I think this is good to go. (And I think the color thing could be done as a second step if necessary -- my other comments are more important, IMHO).

@dmcdougall
Copy link
Member Author

Thanks for your feedback @mdboom. GitHub is great for code review and feedback. I love it.

None of these most recent commits include the colors keyword argument functionality you mentioned. I figured I could do that at a later time.

@mdboom
Copy link
Member

mdboom commented Apr 16, 2012

I think this is ready to merge. Will leave it up for a few more days in case others would like to comment.

@WeatherGod
Copy link
Member

pyplot.py needs update, as well as some docs (front page, what's new and API changes). I haven't looked over the docstrings yet.

@dmcdougall
Copy link
Member Author

I've been trying to figure out what @WeatherGod means by 'pyplot.py needs an update'. I think I figured it out.

I add stackplot to the _plot_commands variable and run boilerplate.py. Then I tested the stackplot function using pylab. I get the following error:

'NoneType' object has no attribute 'pop'.

I then spent an hour trying figure out what was going wrong. Long story short:

  1. stackplot has no argument **kwargs;
  2. as a result, boilerplate.py adds the line: hold = None.pop('hold', None) to the stackplot method in pyplot.py.

How should I deal with this? Deleting it manually is not a solution because any time anybody else runs boilerplate the same thing will happen. Maybe I should just add a **kwargs argument and pass them to all the called Axes.fill_between methods in stackplot. The bonus there is that stackplot would then support things like alpha, linewidth and facecolor. Also, I plan to add support for custom coloring, so it will inevitably be added in the future.

@mdboom
Copy link
Member

mdboom commented Jun 1, 2012

This perhaps should be re-evaluated in relation to the related PR #847.

The example should be converted into a unit test.

Other than that, this looks generally good to me.

@WeatherGod
Copy link
Member

It does appear to need a rebase at the very least. Most likely due to changes in pyplot.py?

@dmcdougall
Copy link
Member Author

You mean you want me to rebase against current master? I can do that. Are there any changes you wish me to make before I rebase?

@WeatherGod
Copy link
Member

Changes before rebasing would only complicate things. There still remains discussion regarding design decisions, but at least we would have a working branch to refer to in this manner.

@dmcdougall
Copy link
Member Author

I've rebased against master.

@pelson
Copy link
Member

pelson commented Jun 18, 2012

@dmcdougall: Unfortunately this will need rebasing again due to some changes to the boilerplate script. It is possible that you will get conflicts there and in the actual pyplot file. The good news is that you can now simply run boilerplate.py and it automatically updates pyplot for you.

Once you have done that, and added the example code as a simple unit test, I will merge the PR. Great work!

Stackplot adds the feature request in issue matplotlib#359
(matplotlib#359) and borrows
heavily from Doug Y'barbo's stackoverflow answer
(http://stackoverflow.com/questions/2225995/how-can-i-create-stacked-line-graph-with-matplotlib).
The data *y* is assumed to be unstacked. This is now explicitly said in
the docstring for the paramater *y*.
stackplot now supports taking either an MxN array of data, stacking
along axis=0, or an arbitrary number of 1xN arrays, stacking them in the
order passed.

No checks are done for the case when no the number of args is zero. An
exception should probably be raised here.

Updated docstring to explain new call signatures.
Stackplot example now reflects usage of variadic calling signature.
…_between().

This is a measure to make boilerplate.py not produce a pyplot.py file
with syntax errors. It's also a sensible course of action considering
new stackplot features will almost certainly require keyword arguments
anyway.
pyplot.py now contains the necessary information to make stackplot work
inside of pyplot.
New keyword argument 'colors' can be passed to allow custom colouring of
the stacked areas in a stacked area plot.
@dmcdougall
Copy link
Member Author

@pelson Thanks for the feedback. I've rebased and saw a conflict in boilerplate.py. I fixed everything. I'm currently at a conference stuck behind a router that blocks all ports except 80 and 443. I will push the changes as soon as I am behind a reasonable router.

@dmcdougall
Copy link
Member Author

Aha! I ninja pushed through a proxy. All sorted now!

@ghost ghost assigned pelson Jul 1, 2012
@pelson
Copy link
Member

pelson commented Jul 5, 2012

@dmcdougall: Ideally, I would just like to get a unittest added and then I can press the big green merge button ;-)

Test images added. Unit test writted and unit test added to default
tests in __init__.py
@dmcdougall
Copy link
Member Author

Done. Let me know if I messed anything up. I followed the guidelines here.

Thanks.

@pelson
Copy link
Member

pelson commented Jul 6, 2012

Don't think you need the stackplot_test_image_pdf.png file.

@dmcdougall
Copy link
Member Author

I have no idea why that was there; the test produced it. Maybe because I am using the pdf backend by default? Anyway, it's gone.

@pelson
Copy link
Member

pelson commented Jul 7, 2012

So the tests, by default, produce a png, a svg and a pdf, but in order to do the test comparison they are rasterised into <fname>_svg.png and <fname>_pdf.png, the latter two don't need to be in the baseline directory. Will merge in 24 hrs.

pelson added a commit to pelson/matplotlib that referenced this pull request Jul 28, 2012
@pelson
Copy link
Member

pelson commented Jul 28, 2012

I merged this manually to mpl. I moved your test into the test_axes module and squashed the commits into one (see 5675ffa).

Thanks for all your work (and patience) on this PR.

@pelson pelson closed this Jul 28, 2012
@pelson pelson mentioned this pull request Aug 19, 2012
@naught101
Copy link

Is there a way to remove the black lines between the polygons?

@naught101
Copy link

Oh, yeah, this will do it:

fbk = {'lw':0.0, 'edgecolor':None}
plt.stackplot(x, y.T, **fbk)

@naught101
Copy link

Stackplot currently takes Y in a transposed format.. should that be fixed?

@jenshnielsen
Copy link
Member

Well transposed or not depends on the layout of your data. This is probably not the best place to discuss such changes. Comments on an old closed issue/PR are likely to be lost. If you want to suggest changes please either use the mailing list or create a new open issue/PR

@naught101
Copy link

Ok, done: #4952

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.

None yet

7 participants