Skip to content

Simplify parsing of errorbar input. #13124

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jan 17, 2019
Merged

Simplify parsing of errorbar input. #13124

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 20 additions & 33 deletions lib/matplotlib/axes/_axes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3108,41 +3108,28 @@ def xywhere(xs, ys, mask):
return xs, ys

def extract_err(err, data):
'''private function to compute error bars

Parameters
----------
err : iterable
xerr or yerr from errorbar
data : iterable
x or y from errorbar
'''
try:
"""
Private function to parse *err* and subtract/add it to *data*.

Both *err* and *data* are already iterables at this point.
"""
try: # Asymmetric error: pair of 1D iterables.
a, b = err
iter(a)
iter(b)
except (TypeError, ValueError):
pass
else:
if np.iterable(a) and np.iterable(b):
# using list comps rather than arrays to preserve units
low = [thisx - thiserr for thisx, thiserr
in cbook.safezip(data, a)]
high = [thisx + thiserr for thisx, thiserr
in cbook.safezip(data, b)]
return low, high
# Check if xerr is scalar or symmetric. Asymmetric is handled
# above. This prevents Nx2 arrays from accidentally
# being accepted, when the user meant the 2xN transpose.
# special case for empty lists
if len(err) > 1:
fe = cbook.safe_first_element(err)
if len(err) != len(data) or np.size(fe) > 1:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the check that len(err) == len(data) is taken care of by safezip, so can be dropped.

raise ValueError("err must be [ scalar | N, Nx1 "
"or 2xN array-like ]")
# using list comps rather than arrays to preserve units
low = [thisx - thiserr for thisx, thiserr
in cbook.safezip(data, err)]
high = [thisx + thiserr for thisx, thiserr
in cbook.safezip(data, err)]
a = b = err # Symmetric error: 1D iterable.
# This could just be `np.ndim(a) > 1 and np.ndim(b) > 1`, except
# for the (undocumented, but tested) support for (n, 1) arrays.
a_sh = np.shape(a)
b_sh = np.shape(b)
if (len(a_sh) > 2 or (len(a_sh) == 2 and a_sh[1] != 1)
or len(b_sh) > 2 or (len(b_sh) == 2 and b_sh[1] != 1)):
raise ValueError(
"err must be a scalar or a 1D or (2, n) array-like")
# Using list comprehensions rather than arrays to preserve units.
Copy link
Member

Choose a reason for hiding this comment

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

Would it be worth it to special-case if v and e are arrays to speed up the calculation? Probably not.

low = [v - e for v, e in cbook.safezip(data, a)]
Copy link
Member

Choose a reason for hiding this comment

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

What units has this been tested with? This is basically the same issue as bar in #12903 so we should make sure unit handling is the same. In particular if v is a datetime, and e a timedelta, I assume this works? What about for pint units? And are we sure this works w/ pandas? Not that I think #12903 checked all those boxes, but I think we should be thinking about how to uniformly handle this case and bar. ie. we now have _convert_dx, and maybe errorbar should use it as well.

Copy link
Contributor Author

@anntzer anntzer Jan 16, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Do you want to first make a PR letting errorbar use _convert_dx? I can rebase this one onto yours after it's merged.

However it's not actually clear to me it's the "same" issue as #12903; in bar() you need to be able to support deunitized widths (because the default, 0.8, is deunitized...) whereas here we can just always assume that the error has the same unit (or a compatible one) as the value.

Copy link
Member

Choose a reason for hiding this comment

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

I agree they are different and maybe using the same helper doesn't make sense. (I actually think defaulting width=0.8 in bar is ambiguous and a mistake, but...).

Tests with datetime-like obects would be helpful. Not sure categoricals need to be tested (when would the error be a +/- f? )

And we should decide if we should add pint to the tests.

Copy link
Contributor Author

@anntzer anntzer Jan 17, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

tbh I don't particularly care about adding tests for units for this PR specifically.

Copy link
Member

Choose a reason for hiding this comment

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

actually I guess thats fair since you didn't change the algorithm...

high = [v + e for v, e in cbook.safezip(data, b)]
return low, high

if xerr is not None:
Expand Down