Deprecate Tick.{gridOn,tick1On,label1On,...} in favor of set_visible. #10088

anntzer · 2017-12-26T03:03:40Z

Visibility attributes should just be set on the underlying artists
(tick.gridline.set_visible, etc.) rather than maintaining yet another
layer of control.

Note that this technically breaks code that overrode the gridOn etc. attributes using properties (such as the old implementation of skewed axes) but I can't think of a reasonable way to emit a warning for these...

PR Summary

PR Checklist

Has Pytest style unit tests
Code is PEP 8 compliant
New features are documented, with examples if plot related
Documentation is sphinx and numpydoc compliant
Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

efiring · 2017-12-27T16:54:45Z

I have mixed feelings about this, so far. The 'tick1On' etc. attributes are ugly but fairly deeply embedded. Is this the time to make such an invasive change? Or would it be just as well to defer it for a more thorough overhaul of the overly complex and sluggish Tick/Axis/Spine system? Maybe we will never get to that, though.

anntzer · 2017-12-27T18:21:27Z

I don't think the system is that deeply embedded (but it has been around for a while, for sure). The deprecation is basically just replacing the attributes by calls to set_visible throughout; the only complication is with code that proxies these attributes using properties (i.e., the skew axes example), but this kind of code is always going to interact in a tricky manner with deprecations (because we arguably can't cover all possible ways in which our attributes have been proxied...).

tacaswell · 2018-01-02T01:38:51Z

I do not disagree it simplifies things, but I am not sure it is worth the disruption in this case.

attn @astrofrog Does this cause issues for WCSAxes?

dopplershift · 2018-03-28T22:00:29Z

The changes for SkewT seem sensible to me. I hadn't seen ExitStack before. Certainly I'm a bit 👍 on having one way to handle things (i.e. set_visible) across the library.

Would this work today if I stole the draw() implementation?

anntzer · 2018-03-28T22:15:59Z

Should work, but haven't tried :)
ExitStack is not really necessary (if you want py2 e.g., although a backport is available too), you can do the same with explicitly storing the original visibility state and restoring it in a finally clause. But I like ExitStacks...

astrofrog · 2018-05-02T21:17:22Z

I don't think this will cause issues for WCSAxes, though it will require some updates in APLpy (https://github.com/aplpy/aplpy/blob/eb09ec39506db2da91f92a383016d1328df731c3/aplpy/colorbar.py#L133) but I can live with that.

anntzer · 2018-07-07T21:14:09Z

Not going to insist on marking this as RC, but still bumping the suggestion up.

tacaswell · 2018-08-27T19:37:36Z

I am ok with this in principle, needs a rebase and version updates.

anntzer · 2018-08-27T20:14:28Z

done

timhoffm · 2018-08-27T20:42:56Z

There are a number of cases (most notably update_position() and draw()) that were previously only executed conditionally on the visibility.

I agree that updating always is the more correct thing to do (otherwise the value is not correct when enabling visibility later on.). On the other hand, I'm a bit concerned of the performance impact. Have you evaluated this?

anntzer · 2018-08-27T21:09:04Z

I haven't, but I also think that it is the correct thing to do. Obviously it would be possible to hide them back again under a check for get_visible() but the semantics would be so-slightly different (if someone was really relying on that) and I don't think it's a good feature to keep anyways...
Can probably do a proper benchmark at some point, if you feel it's necessary.

jklymak

Seems fine, except for the ExitStack context stuff, which I don't follow, and hence is maybe a bit obtuse for a user example.

Does this have a slight behaviour change. Before if we set things while the label was not on it didn't have an affect. Now the label is not visible, but you do change the properties. So if the user turns the labels on again, they will get different results I think with this PR than before. OTOH I think the results they will get w/ this PR are more correct, so I'm OK w/ the change.

jklymak · 2018-09-28T04:24:24Z

examples/specialty_plots/skewt.py

-    def label2On(self, value):
-        self._label2On = value
+    def draw(self, renderer):
+        with ExitStack() as stack:


This is kind of mysterious for an example w/o a comment why you need it.

added an explanation, I hope it helps

jklymak · 2018-10-04T18:27:23Z

CI failures unrelated...

@efiring, you expressed misgivings. Are you placated, or ?? If so, I'll merge...

efiring · 2018-10-04T18:37:09Z

OK to merge once it passes the tests.

jklymak · 2018-10-04T18:51:30Z

@anntzer after a rebase feel free to merge once it passes tests....

Visibility attributes should just be set on the underlying artists (`tick.gridline.set_visible`, etc.) rather than maintaining yet another layer of control.

anntzer · 2018-10-04T21:14:45Z

Done (modulo appveyor which is queued, but there's no reason for it to fail :p).

ImportanceOfBeingErnest · 2019-03-06T19:08:51Z

The label attribute, which was an alias for label1, has been deprecated.

This will hit every call to seaborn.heatmap, which is a pretty common function inside seaborn.

jklymak · 2019-03-06T19:21:13Z

will it be a problem for seaborn to replace by label1?

ImportanceOfBeingErnest · 2019-03-06T19:36:50Z

I mean it used to be a simple alias, self.label = self.label1. So that shouldn't be a problem. I just wanted to make sure we are aware of the implication that once 3.1 is released thousands of seaborn users will get some warning message.

timhoffm · 2019-03-07T00:13:50Z

Good point. We shouldn't do that.

The Tick.label property will be deprecated and begin warning in a future version of Matplotlib. Explicitly using the correct property will avoid this. See matplotlib/matplotlib#10088 and matplotlib/matplotlib#13631

anntzer force-pushed the tick-visibility branch 3 times, most recently from 46dfba6 to d13ade3 Compare December 26, 2017 19:53

anntzer force-pushed the tick-visibility branch 2 times, most recently from 795de46 to 4102b75 Compare February 18, 2018 04:31

anntzer added this to the v3.0 milestone Feb 19, 2018

anntzer force-pushed the tick-visibility branch from 4102b75 to f0fb604 Compare February 19, 2018 05:25

anntzer mentioned this pull request Mar 28, 2018

Refactor of axis.py/ticking should have large performance gains #5665

Open

anntzer force-pushed the tick-visibility branch from f0fb604 to bee5871 Compare March 28, 2018 21:26

dopplershift mentioned this pull request Mar 28, 2018

Rework SkewXTick Unidata/MetPy#792

Closed

anntzer force-pushed the tick-visibility branch 2 times, most recently from e9d013d to 20cb675 Compare May 3, 2018 21:16

anntzer force-pushed the tick-visibility branch from 20cb675 to c60605e Compare July 7, 2018 21:28

tacaswell modified the milestones: v3.0, v3.1 Jul 15, 2018

anntzer mentioned this pull request Jul 18, 2018

Make Figure.frameon a thin wrapper for the patch visibility. #11691

Merged

6 tasks

jklymak added the status: needs review label Aug 20, 2018

dopplershift approved these changes Aug 27, 2018

View reviewed changes

anntzer force-pushed the tick-visibility branch from c60605e to c31cc44 Compare August 27, 2018 20:14

jklymak self-requested a review September 24, 2018 19:38

anntzer force-pushed the tick-visibility branch from c31cc44 to ed0a056 Compare September 24, 2018 19:47

jklymak approved these changes Sep 28, 2018

View reviewed changes

anntzer force-pushed the tick-visibility branch from f14da5b to cefae87 Compare September 28, 2018 05:54

jklymak added the status: needs rebase label Oct 4, 2018

anntzer added 2 commits October 4, 2018 22:33

Deprecate Tick.{gridOn,tick1On,label1On,...} in favor of set_visible.

525dcef

Visibility attributes should just be set on the underlying artists (`tick.gridline.set_visible`, etc.) rather than maintaining yet another layer of control.

Placate flake8.

c61d71d

anntzer force-pushed the tick-visibility branch from cefae87 to c61d71d Compare October 4, 2018 20:34

anntzer removed the status: needs rebase label Oct 4, 2018

jklymak merged commit 13629a4 into matplotlib:master Oct 4, 2018

anntzer deleted the tick-visibility branch October 4, 2018 21:29

anntzer removed the status: needs review label Oct 4, 2018

ksunden mentioned this pull request Nov 19, 2018

BUG: Prevent Tick params calls from overwriting visibility without being told to #12839

Merged

6 tasks

dopplershift mentioned this pull request Dec 27, 2018

Update SkewT implementation Unidata/MetPy#987

Closed

This was referenced Jan 31, 2019

label1On set to true when axis.tick_params(axis='both', which='major', length=5) #13223

Closed

Update and factor out Axis.get_tick_positions. #13335

Merged

tacaswell mentioned this pull request Mar 8, 2019

do not cause warning in seaborn #13623

Closed

tacaswell mentioned this pull request Mar 9, 2019

MNT: tweak access to tick label to avoid warning mwaskom/seaborn#1692

Merged

jklymak mentioned this pull request Sep 4, 2019

Twinning an axis resets tick visibility #15188

Closed

TheAngulion mentioned this pull request Jan 12, 2020

Toggling visibility of axis tick labels causes their destruction #16197

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deprecate Tick.{gridOn,tick1On,label1On,...} in favor of set_visible. #10088

Deprecate Tick.{gridOn,tick1On,label1On,...} in favor of set_visible. #10088

anntzer commented Dec 26, 2017 •

edited

Loading

efiring commented Dec 27, 2017

anntzer commented Dec 27, 2017

tacaswell commented Jan 2, 2018

dopplershift commented Mar 28, 2018

anntzer commented Mar 28, 2018 •

edited

Loading

astrofrog commented May 2, 2018

anntzer commented Jul 7, 2018

tacaswell commented Aug 27, 2018

anntzer commented Aug 27, 2018

timhoffm commented Aug 27, 2018

anntzer commented Aug 27, 2018

jklymak left a comment

jklymak Sep 28, 2018

anntzer Sep 28, 2018

jklymak commented Oct 4, 2018

efiring commented Oct 4, 2018

jklymak commented Oct 4, 2018

anntzer commented Oct 4, 2018

ImportanceOfBeingErnest commented Mar 6, 2019

jklymak commented Mar 6, 2019

ImportanceOfBeingErnest commented Mar 6, 2019

timhoffm commented Mar 7, 2019

Deprecate Tick.{gridOn,tick1On,label1On,...} in favor of set_visible. #10088

Deprecate Tick.{gridOn,tick1On,label1On,...} in favor of set_visible. #10088

Conversation

anntzer commented Dec 26, 2017 • edited Loading

PR Summary

PR Checklist

efiring commented Dec 27, 2017

anntzer commented Dec 27, 2017

tacaswell commented Jan 2, 2018

dopplershift commented Mar 28, 2018

anntzer commented Mar 28, 2018 • edited Loading

astrofrog commented May 2, 2018

anntzer commented Jul 7, 2018

tacaswell commented Aug 27, 2018

anntzer commented Aug 27, 2018

timhoffm commented Aug 27, 2018

anntzer commented Aug 27, 2018

jklymak left a comment

Choose a reason for hiding this comment

jklymak Sep 28, 2018

Choose a reason for hiding this comment

anntzer Sep 28, 2018

Choose a reason for hiding this comment

jklymak commented Oct 4, 2018

efiring commented Oct 4, 2018

jklymak commented Oct 4, 2018

anntzer commented Oct 4, 2018

ImportanceOfBeingErnest commented Mar 6, 2019

jklymak commented Mar 6, 2019

ImportanceOfBeingErnest commented Mar 6, 2019

timhoffm commented Mar 7, 2019

anntzer commented Dec 26, 2017 •

edited

Loading

anntzer commented Mar 28, 2018 •

edited

Loading