Skip to content

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

Merged
merged 2 commits into from
Dec 5, 2024

Conversation

story645
Copy link
Member

@story645 story645 commented Nov 21, 2024

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:

fig, (ax1, ax2) = plt.subplots(nrows=2, figsize=(10, 5))
x = pd.Timestamp('2017-01-01T12')
ax1.axvline(x)
y = "test test"
ax2.axhline(y)
arr = mpatches.ConnectionPatch((x, 0), (0, y), coordsA='data', coordsB='data', axesA=ax1, axesB=ax2)
fig.add_artist(arr)

With the changes in this PR, the ConnectionPatch now renders:
Figure_1

PR checklist

@story645 story645 changed the title ConnectionPatch passes through axes unit information Pass through axes unit information in ConnectionPatch Nov 21, 2024
@story645 story645 force-pushed the connectionpatch_units branch from c91dfb8 to 9b69f8c Compare November 21, 2024 07:08
@story645 story645 force-pushed the connectionpatch_units branch from 9b69f8c to a878579 Compare November 21, 2024 18:57
Comment on lines 4611 to 4612
x = cbook._to_unmasked_float_array(self.convert_xunits(x, axes))
y = cbook._to_unmasked_float_array(self.convert_yunits(y, axes))
Copy link
Member

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?

Copy link
Member Author

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.

@story645 story645 force-pushed the connectionpatch_units branch from a878579 to c8165bf Compare November 22, 2024 19:35
@story645 story645 changed the title Pass through axes unit information in ConnectionPatch BUGFIX: Pass through axes unit information in ConnectionPatch Nov 24, 2024
@story645 story645 force-pushed the connectionpatch_units branch from c8165bf to 3329dd6 Compare November 25, 2024 20:13
Modifies string convertor to return a scaler when value is only one value, as with datetime
@story645 story645 force-pushed the connectionpatch_units branch from 3329dd6 to 227180c Compare November 27, 2024 21:00
@story645 story645 changed the title BUGFIX: Pass through axes unit information in ConnectionPatch BUGFIX: use axes unit information in ConnectionPatch Nov 27, 2024
@ksunden ksunden added this to the v3.11.0 milestone Dec 4, 2024
@story645
Copy link
Member Author

story645 commented Dec 4, 2024

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.

@ksunden ksunden modified the milestones: v3.11.0, v3.10.0 Dec 5, 2024
@QuLogic QuLogic merged commit 915f672 into matplotlib:main Dec 5, 2024
43 checks passed
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Dec 5, 2024
@story645 story645 deleted the connectionpatch_units branch December 5, 2024 20:46
QuLogic added a commit that referenced this pull request Dec 6, 2024
…167-on-v3.10.x

Backport PR #29167 on branch v3.10.x (BUGFIX: use axes unit information in ConnectionPatch )
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants