Fix for #4984 #4985
Fix for #4984 #4985
Conversation
wackywendell
force-pushed
the
errorbarfix
branch
from
9e032e9
to
100248b
Compare
|
Don't use asserts to protect against bad user inputs. Asserts can be optimized away. If the input is bad, then raise a ValueError. |
wackywendell
force-pushed
the
errorbarfix
branch
from
100248b
to
048e8c9
Compare
|
Thank you, that's a good point. Does it look better now? |
| @@ -2825,6 +2825,12 @@ def xywhere(xs, ys, mask): | |||
| in cbook.safezip(x, xerr[1])] | |||
| else: | |||
| # using list comps rather than arrays to preserve units | |||
| if not (len(xerr) == 1 or # scalar | |||
| # or symmetric errorbars, which must be a single list | |||
There was a problem hiding this comment.
This is pretty minor, but I really don't like these long in-logic comments, I find it makes the code hard to read.
There was a problem hiding this comment.
also, the comment about the list comprehensions is now separated from the list comprehensions.
wackywendell
force-pushed
the
errorbarfix
branch
from
048e8c9
to
881316c
Compare
|
Good points, thanks again! Any more? |
wackywendell
force-pushed
the
errorbarfix
branch
from
881316c
to
f875390
Compare
|
Can you add a test of a |
|
Thanks for putting up with us. |
wackywendell
force-pushed
the
errorbarfix
branch
from
f875390
to
696a266
Compare
Hey, thanks for the help and understanding! |
| (len(xerr) == len(x) and not ( | ||
| iterable(xerr[0]) and len(xerr[0]) > 1))): | ||
| raise ValueError("yerr must be a scalar, the same " | ||
| "dimensions as y, or 2xN.") |
There was a problem hiding this comment.
Yikes! That one was pretty dumb. Thanks!
wackywendell
force-pushed
the
errorbarfix
branch
from
696a266
to
14e9a80
Compare
|
No worries. This is why code reviews are done and even the regular devs don't self-merge PRs. This looks fine to me once Travis passes. |
| xerr = 0.1 + yerr | ||
|
|
||
| print(np.shape(y)) | ||
| print(np.shape(yerr)) |
There was a problem hiding this comment.
Tiny issue but I would suggest removing these prints
They will show up in the test output with -s passed to nose without any context
wackywendell
force-pushed
the
errorbarfix
branch
from
14e9a80
to
5f33e7d
Compare
|
Thank you! |
This is a very simple assertion to test for the issue described in #4984, and a test to go with it.