Skip to content

Only do pchanged and set stale when value changes + doc consistency #26326

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
Aug 9, 2023

Conversation

oscargus
Copy link
Member

PR summary

Original purpose was to not call pchanged or set stale to True if the value did not change (some set-methods already do this).

However, I found some doc inconsistencies so did a bit of search-and-replace as well...

PR checklist

self.pchanged()
self.stale = True
Copy link
Contributor

Choose a reason for hiding this comment

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

For a follow-up PR I wonder if we should move self.stale = True into the pchanged definition? It seems like every call to pchanged is followed by self.stale = True...

Copy link
Member Author

Choose a reason for hiding this comment

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

That can for sure be a good idea.

@oscargus oscargus force-pushed the artistprotection branch 3 times, most recently from 87098e3 to 76fcf6e Compare July 17, 2023 12:36
@QuLogic
Copy link
Member

QuLogic commented Jul 18, 2023

Hmm, most of this seems like it should be in #26334?

@oscargus
Copy link
Member Author

Sort of. I stopped adding those here and started a new PR. I can possibly move individual files to that PR instead.

@oscargus
Copy link
Member Author

oscargus commented Jul 19, 2023

I moved all files other than artist.py to #26334.

@tacaswell tacaswell added this to the v3.8.0 milestone Aug 9, 2023
@tacaswell
Copy link
Member

anyone can merge what green

@ksunden ksunden merged commit 7f843bb into matplotlib:main Aug 9, 2023
@oscargus oscargus deleted the artistprotection branch August 9, 2023 20:57
@larsoner larsoner mentioned this pull request Aug 14, 2023
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.

5 participants