Skip to content

Don't warn on grid removal deprecation if grid is hidden #22285

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jan 25, 2022
Merged

Don't warn on grid removal deprecation if grid is hidden #22285

merged 1 commit into from
Jan 25, 2022

Conversation

timhoffm
Copy link
Member

Fixes #21723.

See #21723 (comment)
for a full desrciption of motivation and edge cases.

@timhoffm timhoffm changed the title Don't warn on grid removal deprecation is grid is hidden Don't warn on grid removal deprecation if grid is hidden Jan 22, 2022
@timhoffm timhoffm added this to the v3.5.2 milestone Jan 22, 2022
jklymak
jklymak reviewed Jan 22, 2022
View reviewed changes
Copy link
Member

@jklymak jklymak left a comment

Choose a reason for hiding this comment

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

It seems we need a test that this doesn't break in the future? Clearly we don't test with the grid turned on often enough. Otherwise the change seems fine to me.

@timhoffm
Don't warn on grid removal deprecation is grid is hidden
9e6d723 
Fixes matplotlib#21723.

See matplotlib#21723 (comment)
for a full desciption of motivation and edge cases.
@timhoffm timhoffm force-pushed the grid-removal-deprecation branch from 6408397 to 9e6d723 Compare January 22, 2022 13:30
@timhoffm
Copy link
Member Author

It seems we need a test that this doesn't break in the future?

I have refrained from adding a test because

  • the deprecation will expire for 3.7., which means we can remove that code after the 3.6 release. So it's only a couple of months. It's unlikely that we touch that code again.
  • The worst thing that can happen is a deprecation message issued less or more than optimal.

@jklymak
Copy link
Member

jklymak commented Jan 24, 2022

The suggestion here was more to make sure we had a couple tests with different styles turned on. The failure with the colorbar was pretty basic, and in a well-used style. However, I'm fine wth the idea that is beyond this PR's scope

QuLogic
QuLogic approved these changes Jan 25, 2022 •
edited
Loading
View reviewed changes
@jklymak jklymak merged commit a6da11e into matplotlib:main Jan 25, 2022
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Jan 25, 2022
@jklymak @meeseeksmachine
Backport PR matplotlib#22285: Don't warn on grid removal deprecation …
d634609 
…if grid is hidden
@meeseeksmachine meeseeksmachine mentioned this pull request Jan 25, 2022
Merged
dstansby added a commit that referenced this pull request Jan 25, 2022
@dstansby
Merge pull request #22311 from meeseeksmachine/auto-backport-of-pr-22…
1ccc8cd 
…285-on-v3.5.x

Backport PR #22285 on branch v3.5.x (Don't warn on grid removal deprecation if grid is hidden)
@pllim pllim mentioned this pull request Jan 25, 2022
Closed
@timhoffm timhoffm deleted the grid-removal-deprecation branch January 25, 2022 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@QuLogic QuLogic QuLogic approved these changes

@jklymak jklymak jklymak approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v3.5.2
Development

Successfully merging this pull request may close these issues.

[Bug]: Some styles trigger pcolormesh grid deprecation
3 participants
@timhoffm @jklymak @QuLogic