Skip to content

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

Merged
dstansby merged 1 commit intomatplotlib:masterfrom
tacaswell:mnt_be_nice_about_hold
Sep 27, 2018
Merged

MNT: put back _hold as read-only attribute on AxesBase#12274
dstansby merged 1 commit intomatplotlib:masterfrom
tacaswell:mnt_be_nice_about_hold

Conversation

@tacaswell
Copy link
Copy Markdown
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
Copy Markdown
Member Author

tacaswell commented Sep 25, 2018

attn @dopplershift

@WeatherGod
Copy link
Copy Markdown
Member

WeatherGod commented Sep 25, 2018 via email

@dopplershift
Copy link
Copy Markdown
Contributor

dopplershift commented Sep 25, 2018

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
Copy Markdown
Contributor

eric-wieser commented Sep 26, 2018

Should the attribute raise a deprecation warning?

@tacaswell
Copy link
Copy Markdown
Member Author

tacaswell commented Sep 27, 2018

@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
Copy Markdown
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
Copy Markdown
Contributor

dopplershift commented Sep 28, 2018

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

@efiring
Copy link
Copy Markdown
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
Copy Markdown
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