Fix #21101 Add validator to errorbar method #21266
Conversation
There was a problem hiding this comment.
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 us on gitter for real-time discussion.
For details on testing, writing docs, and our review process, please see the developer guide
We strive to be a welcoming and open project. Please follow our Code of Conduct.
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.
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 timedeltas can be meaningfully compared to zero).
There was a problem hiding this comment.
Thanks @anntzer! Yeah, I added this line to not fail on datetime.timedelta
matplotlib/lib/matplotlib/tests/test_units.py
Line 174 in 6d56592
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.
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.
@anntzer I've added a check for timedelta types. However, I'm a bit uncomfortable with checking only timedelta type. datetime.datetime handles only positive values -> should not be an issue. What about datetime.date?
you can add a datetime to a timedelta
Do you mean, that there is a way to convert anything to timedelta and check it? If yes, could you elaborate a bit more?
There was a problem hiding this comment.
I agree the special-casing is a bit annoying, I don't have any good solution to offer right now though.
|
Thanks @Kislovskiy, and congratulations on your first contribution to Matplotlib. We hope to see you again. |
Fix matplotlib#21101 Add validator to errorbar method
Fix #21101 Add validator to errorbar method
PR Summary
The PR adds a validator for
xerrandyerrarguments in errorbar method to fix #21101.It brings implementation compliant with the documentation:
source: https://github.com/matplotlib/matplotlib/blob/master/lib/matplotlib/axes/_axes.py#L3183
PR Checklist
pytestpasses).flake8on changed files to check).flake8-docstringsand 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).