Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Fix/restore secondary axis support for Transform-type functions#27267

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

Conversation

zachjweiner
Copy link
Contributor

PR summary

Closes#25691 and adds a test.

First contribution to matplotlib, so please let me know if I've missed anything.

PR checklist

Copy link

@github-actionsgithub-actionsbot left a 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.

if (isinstance(functions, tuple) and len(functions) == 2 and
callable(functions[0]) and callable(functions[1])):
# make an arbitrary convert from a two-tuple of functions
# forward and inverse.
self._functions = functions
elif isinstance(functions, Transform):
self._functions = (
functions.transform, functions.inverted().transform)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

This is dangerous, asTransform.inverted returns acopy with a snapshot of the transform, and thus won't be in sync if the input transform changes. Instead, you'd probably want (identity - input).

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I agree, but is it preferable to implicitly allow arguments to be mutated after the fact as a side effect at all? I.e., would a better way to ensure consistency be to simply copy the passed transform to begin with? Let me know which way to go (not sure if there's precedent here)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Yes, we tend to be lazily evaluated, e.g., if we hadAxes.transData here, it might change depending on limits (though of course that specific transform won't work here, being 2D, and as you've noted above this argument is 1D.)

Copy link
ContributorAuthor

@zachjweinerzachjweinerNov 14, 2023
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Actually, I'm not finding that to be the case (that subtraction avoids inconsistency upon mutation), at least with theTranslation class I defined:

importmatplotlib.pyplotaspltimportmatplotlib.transformsasmtransformsclassTranslation(mtransforms.Transform):input_dims=1output_dims=1def__init__(self,dx):self.dx=dxdeftransform(self,values):returnvalues+self.dxdefinverted(self):returnTranslation(-self.dx)forward=Translation(1)backward=mtransforms.IdentityTransform()-forwardprint(forward.transform(1),backward.transform(1))forward.dx=2print(forward.transform(1),backward.transform(1))

prints

2 03 0

ReviewingTransform.__sub__, I can't actually say I understand how lazy inversion ever occurs... let me know what you think.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

But your test is wrong? The second printout should be3 -1, ifbackward andforward are linked (which is what we want here, since the user only supplied the forward transform.)

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

That's my point -forward andIdentityTransform() - forward are not linked, and I don't see any mechanism for that operation being lazy inTransform.__sub__.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Oh, I see what you're saying now. Yes, I misremembered what happened there. What you will have to do is put alambda in that calculates the inverse when it's called.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Clever -2803407

@tacaswelltacaswell added this to thev3.9.0 milestoneDec 12, 2023
@tacaswell
Copy link
Member

The coverage drop is because it is missing some of the uploads (it is only seeing 5, should be atlesat 9).

@tacaswelltacaswell merged commitb2e3e8b intomatplotlib:mainDec 12, 2023
@tacaswell
Copy link
Member

Thank you for you work@zachjweiner and congratulations on your first merged PR to Matplotlib 🎉

We hope to hear from you again.

@zachjweiner
Copy link
ContributorAuthor

Cheers, thanks! (and thanks@QuLogic for iterating on this!)

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@QuLogicQuLogicQuLogic left review comments

@github-actionsgithub-actions[bot]github-actions[bot] left review comments

@jklymakjklymakjklymak approved these changes

Assignees
No one assigned
Projects
Milestone
v3.9.0
Development

Successfully merging this pull request may close these issues.

[Bug]: Secondary axis does not support Transform as functions
5 participants
@zachjweiner@tacaswell@QuLogic@jklymak@melissawm

[8]ページ先頭

©2009-2025 Movatter.jp