-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
BUGFIX: use axes unit information in ConnectionPatch #29167
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
c91dfb8
to
9b69f8c
Compare
9b69f8c
to
a878579
Compare
lib/matplotlib/patches.py
Outdated
x = cbook._to_unmasked_float_array(self.convert_xunits(x, axes)) | ||
y = cbook._to_unmasked_float_array(self.convert_yunits(y, axes)) |
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.
If we want to use an explicit Axes
, do we need to go through Artist.convert_xunits
(with the addition of the axes
parameter), or instead call Axes.xaxis.convert_units
ourselves with the correct axes
?
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 it this way to keep the consistency w/ the other artists but I think your suggestion is more straightforward so I'll make the changes in the morning if there aren't objections.
a878579
to
c8165bf
Compare
c8165bf
to
3329dd6
Compare
Modifies string convertor to return a scaler when value is only one value, as with datetime
3329dd6
to
227180c
Compare
Can this get milestoned to the next micro release since the functionality it's implementing was supposed to exist (going by the existing do nothing calls out to the convertors)? Only reason I didn't is b/c that milestone isn't listed. |
…167-on-v3.10.x Backport PR #29167 on branch v3.10.x (BUGFIX: use axes unit information in ConnectionPatch )
This PR lets connection patch use the convertor info from the axes that's already being passed in for "data" to support unitized data. I think the original behavior is a bug b/ the original code called the artist convertors but they didn't do anything b/c the artist convertors are looking up the artist axes which doesn't exist b/c connection patch is a figure level artist.
Modifies string convertor to return a scaler when input is only one value b/c that's what the connection patch transform function expects. This is inspired by the datetime convertor, which already behaves this way.
Updated summary based on @QuLogic's review to reflect current implementation/behavior.
This PR is motivated by the following code, which on main draws the respective plots but crashes at rendering the connection patch because it doesn't know about the convertors:
With the changes in this PR, the ConnectionPatch now renders:

PR checklist