Document change of label visibility on shared axes #10981

ImportanceOfBeingErnest · 2018-04-07T23:02:57Z

PR Summary

In #8678 internal use of Axis.set_tick_params has been introduced to some methods previously using the ticklabels properties themselves. This change has led to the labels of shared axes not be turned invisible, but instead not being created at all. Previously working ways to turn the labels visible again are therefore failing, see Get ticklabels back on shared axis #10911.

This PR partially fixes #10911 in the sense of creating a what's new entry for this change and updating the documentation of sharex.

If possible it should be backported to 2.1.0 where this change was silently introduced.

Note that further actions might be required to fully introduce set_tick_params as viable alternative to setting individual artist properties. Those go beyond the scope of this PR and should probably be introduced as new features.

PR Checklist

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 · 2018-04-08T00:33:05Z

I don't see the point of backporting to the 2.1 line; development there is closed. This would be backported to 2.2.x so as to land in 2.2.3.

efiring

Thank you for doing this so promptly. I have one recommendation and a minor correction.

efiring · 2018-04-08T00:35:52Z

doc/api/api_changes.rst

+hide tick labels instead of setting the visibility on the tick label objects.
+This improves overall performance and fixes some issues.
+As a consequence, in case those labels ought to be shown, `set_tick_params`
+needs to be used, e.g. `ax.xaxis.set_tick_params(labelbottom=True)`.


Here and in similar cases below: Is it worth adding the shorter alternative, ax.tick_params(labelbottom=True)? Or giving that instead of the lower-level one? That would be my preference. I think it is easier for users to work with the single point of entry for manipulating tick-related things.

efiring · 2018-04-08T00:39:35Z

lib/matplotlib/figure.py

-              axis object:
+        squeeze : bool, optional, default: True
+            - If True, extra dimensions are squeezed out from the returned Axes
+              object:


Actually, dimensions are squeezed out of the ndarray of Axes objects, not out of any such object.

ImportanceOfBeingErnest · 2018-04-08T01:06:08Z

I'm not sure about the policy here; my thinking was simply if something got introduced in version x.y, the documentation of x.y should have it included, not the documentation of (x+1).(y+3). Of course this would only make sense if the respective documentation is then also rebuilt and put online, not sure if this would be the case.

efiring · 2018-04-08T01:54:13Z

I don't think the documentation of any earlier version is going to be rebuilt--only the docs of future releases. The best you can do is note both the version when the change occurred, and the version when the documentation was updated accordingly.

efiring · 2018-04-08T01:56:10Z

doc/users/prev_whats_new/whats_new_2.1.0.rst

+Internally, :func:`~matplotlib.axis.Axis.set_tick_params` is now used to
+hide tick labels instead of setting the visibility on the tick label objects.
+This improves overall performance and fixes some issues.
+As a consequence, in case those labels ought to be shown, `set_tick_params`


Here also, tick_params.

In the PR that changed that, they use Axis.set_tick_params

I know, but saying "use set_tick_params as in ax.tick_params(labelbottom=True) is likely to make people think there is a typo somewhere. What's really going on internally is that the Tick.label1On parameter is being set, and this can be done at any point in a chain of calls starting with ax.tick_params.

I think the problem is actually starting with the first line; the use of Axis.set_tick_params is irrelevant, and the point is not that tick labels are being hidden in a different way when subplots are shared, but that instead of being flagged as invisible, they are simply not drawn at all. The user can turn label drawing on or off via, e.g., ax.tick_params(labelbottom=True).

efiring · 2018-04-08T01:57:08Z

lib/matplotlib/figure.py

            - If True, extra dimensions are squeezed out from the returned
-              axis object:
+              array of Axes object:


Axes objects (plural)

efiring · 2018-04-08T01:57:47Z

lib/matplotlib/pyplot.py

-        - If True, extra dimensions are squeezed out from the returned Axes
-          object:
+        - If True, extra dimensions are squeezed out from the returned
+          array of Axes object:


I really shouldn't have touched this squeeze parameter at all. 😆

Anyways, as I understand the sentence it is "Extra dimensions are squeezed out from the returned
object." (singular)
Now which object is that more specifically? It is an (array of Axes)-object.

If that should not be the case, could you please suggest the propper wording, because not being a native speaker, I wouldn't know how to write this differently.

My comment was unclear. I meant "array of Axes object" -> "array of Axes objects".

In that case my above comment would still apply. subplots returns one single array. That array is one single object.

But I now understand the issue. So I guess both ways are correct, depending on what you want to say.

This can be an (array of Axes)-object.
Or it can be an array of (Axes-objects).

What about leaving the object out completely?

I’d say “extra dimensions from the two-d array of axes are squeezed out.”

ImportanceOfBeingErnest · 2018-04-08T02:05:16Z

I see. Do you think this to be necessary? (It would look strange in the middle of a docstring, right?)

jklymak · 2018-04-08T02:12:54Z

I think he just means in the API change note.

...but - is it ok to back-change these documents this way? I'd not expect to see the API changes and whats new be different between different versions of the docs.

efiring · 2018-04-08T02:22:30Z

Since the docs reside in the same repo as the code, I don't see how it would make sense to update the docs in a closed branch, without reopening it and releasing with a new tag; the docs would be out of sync with the repo as of the last tagged release on that branch. We aren't going to have any more tags and releases on 2.1, so I think we are stuck. It won't be the only less-than-perfect aspect of the docs for 2.1, or for any other version, for that matter.

jklymak · 2018-04-08T02:25:37Z

Yeah, I meant the other way, that these should be just put at the top of the whats new and API changes

ImportanceOfBeingErnest · 2018-04-16T20:43:46Z

I'm not 100% sure if I follow the arguments here. Is there anything more I should do for this PR?

jklymak · 2018-04-24T00:20:01Z

@efiring is this OK w/ you?

timhoffm · 2018-04-24T23:26:48Z

Still not sure about the branch policy, but AFAIK this has to go into v2.2.3 instead of v2.2.-doc because it changes .py files (even if those changes are only in docstrings). Please correct me, if I'm wrong here.

jklymak · 2018-04-24T23:30:40Z

That’s correct (though I didn’t realize this until yesterday)

Backport PR #10981 on branch v2.2.x

ImportanceOfBeingErnest mentioned this pull request Apr 7, 2018

Get ticklabels back on shared axis #10911

Closed

document labelvisibility on shared axes change

72d20b0

ImportanceOfBeingErnest force-pushed the doc-labelvisibility-on-shared-axes branch from 3ada949 to 72d20b0 Compare April 7, 2018 23:32

jklymak added the Documentation label Apr 8, 2018

jklymak added this to the v2.2-doc milestone Apr 8, 2018

efiring requested changes Apr 8, 2018

View reviewed changes

efiring reviewed Apr 8, 2018

View reviewed changes

document labelvisibility on shared axes change (rev1)

8977953

ImportanceOfBeingErnest force-pushed the doc-labelvisibility-on-shared-axes branch from be0f403 to 8977953 Compare April 8, 2018 20:14

efiring approved these changes Apr 24, 2018

View reviewed changes

timhoffm modified the milestones: v2.2-doc, v2.2.3 Apr 24, 2018

timhoffm approved these changes Apr 24, 2018

View reviewed changes

jklymak merged commit 0984694 into matplotlib:master Apr 25, 2018

lumberbot-app bot pushed a commit that referenced this pull request Apr 25, 2018

Backport PR #10981: Document change of label visibility on shared axes

e26a517

lumberbot-app bot mentioned this pull request Apr 25, 2018

Backport PR #10981 on branch v2.2.x #11120

Merged

jklymak added a commit that referenced this pull request Apr 26, 2018

Merge pull request #11120 from matplotlib/auto-backport-of-pr-10981

a421abf

Backport PR #10981 on branch v2.2.x

ImportanceOfBeingErnest deleted the doc-labelvisibility-on-shared-axes branch May 10, 2018 15:50

anntzer mentioned this pull request Oct 11, 2018

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

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Document change of label visibility on shared axes #10981

Document change of label visibility on shared axes #10981

ImportanceOfBeingErnest commented Apr 7, 2018 •

edited

Loading

efiring commented Apr 8, 2018

efiring left a comment

efiring Apr 8, 2018

efiring Apr 8, 2018

ImportanceOfBeingErnest commented Apr 8, 2018

efiring commented Apr 8, 2018

efiring Apr 8, 2018

ImportanceOfBeingErnest Apr 8, 2018

efiring Apr 8, 2018

efiring Apr 8, 2018

efiring Apr 8, 2018

ImportanceOfBeingErnest Apr 8, 2018

efiring Apr 8, 2018

ImportanceOfBeingErnest Apr 8, 2018

ImportanceOfBeingErnest Apr 8, 2018

jklymak Apr 8, 2018

ImportanceOfBeingErnest commented Apr 8, 2018

jklymak commented Apr 8, 2018

efiring commented Apr 8, 2018

jklymak commented Apr 8, 2018

ImportanceOfBeingErnest commented Apr 16, 2018

jklymak commented Apr 24, 2018

timhoffm commented Apr 24, 2018 •

edited

Loading

jklymak commented Apr 24, 2018

Document change of label visibility on shared axes #10981

Document change of label visibility on shared axes #10981

Conversation

ImportanceOfBeingErnest commented Apr 7, 2018 • edited Loading

PR Summary

PR Checklist

efiring commented Apr 8, 2018

efiring left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ImportanceOfBeingErnest commented Apr 8, 2018

efiring commented Apr 8, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ImportanceOfBeingErnest commented Apr 8, 2018

jklymak commented Apr 8, 2018

efiring commented Apr 8, 2018

jklymak commented Apr 8, 2018

ImportanceOfBeingErnest commented Apr 16, 2018

jklymak commented Apr 24, 2018

timhoffm commented Apr 24, 2018 • edited Loading

jklymak commented Apr 24, 2018

ImportanceOfBeingErnest commented Apr 7, 2018 •

edited

Loading

timhoffm commented Apr 24, 2018 •

edited

Loading