FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Simplify parsing of errorbar input. by anntzer · Pull Request #13124 · matplotlib/matplotlib · GitHub

  1. matplotlib
  2. matplotlib

Repository navigation

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
jklymak merged 1 commit into from
Jan 17, 2019
Conversation 13 Commits 1 Checks 0 Files changed
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 .py  (1) All 1 file type selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
53 changes: 20 additions & 33 deletions lib/matplotlib/axes/_axes.py
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
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:

anntzer Jan 6, 2019

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

timhoffm Jan 7, 2019

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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)]

jklymak Jan 16, 2019

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

anntzer Jan 16, 2019 •
edited
Loading

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

jklymak Jan 17, 2019

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

anntzer Jan 17, 2019 •
edited
Loading

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

jklymak Jan 17, 2019

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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
Back | FazBrowse Home | New Git URL