Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Fix #21101 Add validator to errorbar method#21266
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thank you for opening your first PR into Matplotlib!
If you have not heard from us in a while, please feel free to ping@matplotlib/developers
or anyone who has commented on the PR. Most of our reviewers are volunteers and sometimes things fall through the cracks.
You can also join uson gitter for real-time discussion.
For details on testing, writing docs, and our review process, please seethe developer guide
We strive to be a welcoming and open project. Please follow ourCode of Conduct.
Uh oh!
There was an error while loading.Please reload this page.
lib/matplotlib/axes/_axes.py Outdated
try: | ||
if np.any(array < 0): | ||
return True | ||
except TypeError: # Don't fail on 'datetime.*' types |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
It's unclear to me why we wouldn't want to fail with datetime types. After all you can't add datetimes so using them as x/yerr should fail, afaict (you can add a datetime to a timedelta, but timedeltascan be meaningfully compared to zero).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thanks@anntzer! Yeah, I added this line to not fail ondatetime.timedelta
ax.errorbar(x,y,timedelta(days=0.5)) |
I've assumed that it's always positive. Let me see if there is a way to check that they are "meaningful compared to zero".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Ah, I was thinking about np.timedelta, not datetime.timedelta, sorry for the careless reading (np.timedelta(...) > 0
works). Well, I guess my point remains: it would be nice to have the check also for datetime.timedelta inputs, but I don't know how easy that is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@anntzer I've added a check for timedelta types. However, I'm a bit uncomfortable with checking onlytimedelta
type.datetime.datetime
handles only positive values -> should not be an issue. What aboutdatetime.date
?
you can add a datetime to a timedelta
Do you mean, that there is a way to convert anything totimedelta
and check it? If yes, could you elaborate a bit more?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I agree the special-casing is a bit annoying, I don't have any good solution to offer right now though.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Thanks@Kislovskiy, and congratulations on your first contribution to Matplotlib. We hope to see you again. |
Fixmatplotlib#21101 Add validator to errorbar method
Fix#21101 Add validator to errorbar method
PR Summary
The PR adds a validator for
xerr
andyerr
arguments in errorbar method tofix#21101.It brings implementation compliant with the documentation:
source:https://github.com/matplotlib/matplotlib/blob/master/lib/matplotlib/axes/_axes.py#L3183
PR Checklist
pytest
passes).flake8
on changed files to check).flake8-docstrings
and runflake8 --docstring-convention=all
).doc/users/next_whats_new/
(follow instructions in README.rst there).doc/api/next_api_changes/
(follow instructions in README.rst there).