Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
[TST] Added test_axlines in test_datetime.py#27326
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
base:main
Are you sure you want to change the base?
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 week or so, please leave a new comment below and that should bring it to our attention. 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.
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.
This has revealed thataxline doesnot support units currently, but I think itcould with relative ease, see my other comment for more details
Since it does not support units currently, we are not actually testing the intended code paths here, so it shouldn't be merged as is.
| values = list(range(1, 32)) | ||
| ax1.plot(dates, values, 'ro') | ||
| ax1.axline((date2num(dates[0]), values[0]), (date2num(dates[-1]), values[-1]), |
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.
We specifically donot want to usedate2num in these test cases. the fact that it is erroring when handeddatetime objects is precisely the kind of thing we wanted to find out by implementing these tests.
The following patch gets most of the way to fixing it such that you can actually pass datetime objects (though I think it may need some consideration regarding what to do if a non-data transform is provided, or whether "slope" is something that can be meaningfully provided with unitful data):
diff --git a/lib/matplotlib/axes/_axes.py b/lib/matplotlib/axes/_axes.pyindex 9199f1cafa..524c1ae06a 100644--- a/lib/matplotlib/axes/_axes.py+++ b/lib/matplotlib/axes/_axes.py@@ -919,6 +919,9 @@ class Axes(_AxesBase): self.get_yscale() != 'linear'): raise TypeError("'slope' cannot be used with non-linear scales")+ xy1 = tuple(self._process_unit_info([("x", xy1[0]), ("y", xy1[1])]))+ if xy2 is not None:+ xy2 = tuple(self._process_unit_info([("x", xy2[0]), ("y", xy2[1])])) datalim = [xy1] if xy2 is None else [xy1, xy2] if "transform" in kwargs: # if a transform is passed (i.e. line points not in data space),
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 did the minimal change here to get the point-point version working with the default transform (data space). I do think that the final version would need at least some more.
- passing
kwargsin - Maybe pushing the unit transforming down into
mlines.AxLine? (trying to trace how baseLine2Ddoes it, if that does it in the class, I'd probably follow suit forAxLine, but not super clear on that, actually...AxLineinherits fromLine2D) - some more thought about
slopemay be good... it is already disallowed with non-linear scales, may be best to just disallow it with unitful data or declare it to be in explicitly "unitless data space" (currently it states that it is in 'data coordinates'). I think it may actually work out for at least the case of having the same units on both axes (i.e. the slope is unitless then) or possibly also with full unit libraries, though possibly would require pushing units into theAxLineclass for it to work.
PR summary
This PR adds a smoke test for Axes.axlines in lib/matplotlib/tests/test_datetime.py. This PR closes one of the items listed in the issue#26864.
Smoke test Plot:
PR checklist