ENH: Add baseline feature to stackplot. #1517
Conversation
|
To plot an nice example use: def layers(n, m):
def bump(a):
x = 1 / (.1 + r.random())
y = 2 * r.random() - .5
z = 10 / (.1+ r.random())
for i in range(m):
w = (i / float(m) - y) * z
a[i] += x * np.exp(-w*w)
a=np.zeros((m, n))
for i in range(n):
for j in range(5):
bump(a[:,i])
return a
d=layers(3,100)
clf()
subplot(221)
stackplot(gca(), range(100), d.T, baseline='zero')
subplot(222)
stackplot(gca(), range(100), d.T, baseline='sym')
subplot(223)
stackplot(gca(), range(100), d.T, baseline='wiggle')
subplot(224)
stackplot(gca(), range(100), d.T, baseline='weighted_wiggle') |
|
|
||
| # Color between array i-1 and array i | ||
| for i in xrange(len(y)-1): | ||
| r.append(axes.fill_between(x, y_stack[i,:], y_stack[i+1,:], facecolor=axes._get_lines.color_cycle.next(), **kwargs)) | ||
| return r | ||
| return r |
There was a problem hiding this comment.
You removed (probably accidentally) the new line at the end of this file. Could you add it back in?
|
Thanks @Tillsten. I haven't had a chance to play with this yet, I'll give more detailed feedback once I have. At the very least you should check this code adheres to the PEP8 standard, and you should add some image comparison tests for the new functionality. |
|
@dmcdougall I don't see any PEP8 violations, except the newline issue, maybe you can give me a hint? I have some questions about the image comparison: |
|
@Tillsten I wasn't saying there was any, I was just asking you to check :) When you write the test, run it. The test will fail because there is no baseline image to compare the output images to. The output images from that test run will be used as the baseline images for subsequent tests. I'm not sure that was entirely clear. Let me know if you need more help. Check out the coding guideline if you haven't already. |
|
@dmcdougall After my last disastrous PR i had my eye on PEP8 :). So if there no baseline image, a new one will be made? So i have just to make btw, the link to http://matplotlib.org/devel/testing.html#testing does not work. |
|
Hmm, it seems i can not run the tests without building matplotlib itself, which is hard on my windows machine. I get . Can somebody else check and upload the baseline image? |
|
@dmcdougall except the baseline image issue, i think this one is ready to merge |
|
anything i can do? |
| first_line = center - 0.5 * total | ||
| y_stack += first_line | ||
| else: | ||
| raise ValueError('unknown baseline method') |
There was a problem hiding this comment.
As part of this exception message, I would include what value was unknown.
There was a problem hiding this comment.
Is there any kind of standard message for this kind of error?
There was a problem hiding this comment.
No there is no 'standard' message. But a user could easily run into this error by, for example, making a typo in the baseline method name. Perhaps something more informative would be:
'Unknown baseline method %s' % baseline
Or:
'Baseline method %s not recognised. Expected 'zero', 'sym', 'wiggle' or 'weighted_wiggle'
Note, I am not necessarily suggesting the accepted baseline methods are hard-coded into the exception message. There's probably a better way to do it.
There was a problem hiding this comment.
@dmcdougall I think putting the acceptable methods into the message is fine in this case. They will be quite stable and if another method is added it is only a small change.
|
I removed the testimage (because i can't build master under my windows installtion) for now to make this PR ready |
|
I suspect the travis failure is not my fault? |
|
Yes -- the Travis failure is a false negative -- that happens all too often unfortunately. I'd prefer to get the test in here before we merge -- I'll try to look into this later today. If we merge without it, let's at least create another issue to track it so we don't forget to put it in. |
|
|
||
| # Color between x = 0 and the first array. | ||
| r.append(axes.fill_between(x, 0, y_stack[0, :], | ||
| facecolor=axes._get_lines.color_cycle.next(), **kwargs)) | ||
| r.append(axes.fill_between(x, first_line, y_stack[0,:], |
There was a problem hiding this comment.
PEP8 compliance: there is a space missing between , and :
|
@Tillsten I've written down the pep8 problems. In order to more easily see those, you can use the tool pep8, or the tool flake8 (I use flake8 because it also underlies pyflakes problems, which is very useful). If you use vim or emacs, you can easily have flake8 run automatically on your code. |
|
@NelleV i used a pyflakes, but i think i lost my last (only pep8) commit on rebase. |
| @@ -25,6 +25,16 @@ def stackplot(axes, x, *args, **kwargs): | |||
|
|
|||
| Keyword arguments: | |||
|
|
|||
| *baseline* : ['zero', 'sym', 'wiggle', 'weighted_wiggle'] | |||
| Method use to calculate to baseline. 'zero' is just a | |||
There was a problem hiding this comment.
Maybe I am misunderstanding, but shouldn't it be "Method used to calcule the baseline."?
|
This needs a test and a CHANGELOG entry. |
|
@mdboom This had a test, my problem is that i am under windows and i am not able to run master. So uploading a |
|
@Tillsten I can make a PR against your branch to add a test for you. Or I can fork your branch directly and create a new PR. Any preference? |
|
@dmcdougall Just fork it (and possible take care of the spelling errors). i am not at the right pc at the moment. |
|
See #1671. |
|
@dmcdougall Thanks! closing this one. |
This PR enhances the stacked plot method with some three addition baseline calculations.
All follows from:
http://www.leebyron.com/else/streamgraph/
see the code (bsd) and the paper there.