Skip to content

MNT: put back _hold as read-only attribute on AxesBase #12274

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
Sep 27, 2018
Merged

MNT: put back _hold as read-only attribute on AxesBase #12274

merged 1 commit into from
Sep 27, 2018

Conversation

tacaswell
Copy link
Member

@tacaswell tacaswell commented Sep 25, 2018 •
edited
Loading

This is to un-break the current version of cartopy.

@tacaswell
MNT: put back _hold as read-only attribute on AxesBase
239a28b 
This is to un-break the current version of cartopy.
@tacaswell tacaswell added this to the v3.0.x milestone Sep 25, 2018
@tacaswell
Copy link
Member Author

attn @dopplershift

@WeatherGod
Copy link
Member

WeatherGod commented Sep 25, 2018 via email

@dopplershift
Copy link
Contributor

The issue is that pcolormesh in the current Cartopy is broken by _hold disappearing. There’s no technical question here (indeed Cartopy will actually likely eliminate the offending code in its next release), just that matplotlib will have a bug fix release out soonish and would be nice to not break them.

@eric-wieser
Copy link
Contributor

Should the attribute raise a deprecation warning?

@tacaswell
Copy link
Member Author

@eric-wieser I lean towards no, because there is nothing that users of the current version of cartopy can do about this and cartopy now knows about this and plans to get rid of it.

dstansby
dstansby approved these changes Sep 27, 2018
View reviewed changes
Copy link
Member

@dstansby dstansby left a comment

Choose a reason for hiding this comment

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

I think this is fine without a warning; I think it is generally understood that we are free to do whatever we want with "private" functions/methods/classes that have an underscore at the beginning, and that external libraries shouldn't be using them.

@dstansby dstansby merged commit f4a7fea into matplotlib:master Sep 27, 2018
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Sep 27, 2018
@dstansby
Backport PR matplotlib#12274: MNT: put back _hold as read-only attr…
45335e8 
…ibute on AxesBase
@meeseeksmachine meeseeksmachine mentioned this pull request Sep 27, 2018
Merged
@tacaswell tacaswell deleted the mnt_be_nice_about_hold branch September 27, 2018 11:47
jklymak added a commit that referenced this pull request Sep 27, 2018
@jklymak
Merge pull request #12305 from meeseeksmachine/auto-backport-of-pr-12…
5fa5cab 
…274-on-v3.0.x

Backport PR #12274 on branch v3.0.x (MNT: put back `_hold` as read-only attribute on AxesBase)
@dopplershift
Copy link
Contributor

So, how do we feel about putting back ishold()? The lack of this has broken delaunay_plot_2d in scipy.spatial.

@efiring
Copy link
Member

efiring commented Oct 1, 2018

No, I am strongly opposed to putting back ishold().

@anntzer anntzer mentioned this pull request Oct 3, 2018
Closed
@megies
Copy link
Contributor

megies commented Oct 29, 2018

See matplotlib/basemap#435

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

@dopplershift dopplershift dopplershift approved these changes

@jklymak jklymak jklymak approved these changes

@dstansby dstansby dstansby approved these changes

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

Successfully merging this pull request may close these issues.
8 participants
@tacaswell @WeatherGod @dopplershift @eric-wieser @efiring @megies @jklymak @dstansby