8000 Support removing inner ticks in label_outer() by timhoffm · Pull Request #24376 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]
Skip to content

Support removing inner ticks in label_outer() #24376

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
Aug 9, 2023
Merged

Support removing inner ticks in label_outer() #24376

merged 1 commit into from
Aug 9, 2023

Conversation

timhoffm
Copy link
Member
@timhoffm timhoffm commented Nov 6, 2022

Up to now label_outer(), only affects tick labels, but not ticks. This introduces an optional parameter remove_inner_ticks, which removes the inner ticks if the corresponding tick labels are removed.

Example / Motivation:

x = np.linspace(0, 2 * np.pi, 100)

fig, axs = plt.subplots(2, 2, sharex=True, sharey=True,
                        gridspec_kw=dict(hspace=0.05, wspace=0.03))

axs[0, 0].plot(x, np.sin(x))
axs[0, 1].plot(x, np.cos(x))
axs[1, 0].plot(x, -np.cos(x))
axs[1, 1].plot(x, -np.sin(x))

for ax in axs.flat:
    ax.grid(color='0.9')
    ax.label_outer()

image

It's quite cumbersome to remove the ticks as well (same logic as a for the tick labels).

with ax.label_outer(remove_inner_ticks=True) this becomes trivial

image

@timhoffm timhoffm added this to the v3.7.0 milestone Nov 6, 2022
@timhoffm timhoffm mentioned this pull request Nov 6, 2022
Merged
story645
story645 reviewed Nov 6, 2022
View reviewed changes
lib/matplotlib/axes/_base.py

Parameters
----------
remove_inner_ticks : bool, default: False
Copy link
Member

Choose a reason for hiding this comment

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

If we expect that this will be the desired behavior, what's the cost of making this default to true and add an api change?

Copy link
Member

Choose a reason for hiding this comment

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

we could set it to None for a release and warn (pass the input to silence it), but that would make it a bit annoying to support multiple versions of Matplotlib (as old versions won't even take the input.

I think if we want to flip the default behavior we should have this option available for a few releases and then go through the above dance when the back-compat story is easier for users.

Copy link
Member

Choose a reason for hiding this comment

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

I don't mind adding this as an option, but I pretty strongly feel it should not be the default. I rarely use gridlines, and having the tick nearby makes it easier to quantify by eye.

lib/matplotlib/axes/_base.py
Parameters
----------
remove_inner_ticks : bool, default: False
If True, remove the inner ticks as well (not only tick labels).
Copy link
Member

Choose a reason for hiding this comment

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

needs a versionadded I think?

tacaswell
tacaswell reviewed Nov 6, 2022
View reviewed changes
lib/matplotlib/tests/test_subplots.py
assert tick.tick1line.get_visible() == visible
for ax, y_visible, in zip(axs, y_visible):
for tick in ax.yaxis.get_major_ticks():
assert tick.tick1line.get_visible() == visible
Copy link
Member

Choose a reason for hiding this comment

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

I am very confused by coverage claiming this line is not run, the only thing that makes sense is if in every case where we test this ax.yaxis.get_major_ticks() is an empty list?

Do we need to force a draw at the top of this checker?

@story645
Copy link
Member
story645 commented Nov 6, 2022

I think it's a good idea and have resigned myself to the fact that the very long keyword argument name makes sense.

@jklymak
Copy link
Member
jklymak commented Nov 7, 2022

Note that if you call sharex=True and sharey=True, you do not need to call ax.label_outer to get the current behaviour. This almost says to me that we need a standalone method ax.tick_outer() that does what this kwarg is doing. You could also imagine subplots and friends getting a tick_outer kwarg.

@timhoffm timhoffm marked this pull request as draft November 7, 2022 09:43 8000
@timhoffm
Copy link
Member Author
timhoffm commented Nov 7, 2022

I'll think about this a bit more. - Maybe this is a reasonable extension (If I want to configure tick label visibility, I may optionally want the same for the ticks themselves. Maybe we need something fundamentally better to configure ticks, ticklabels and spines on grid layouts.

@tacaswell
Copy link
Member

Given

I'll think about this a bit more.

I'm going to push this to 3.8 unless @timhoffm disagrees.

@tacaswell tacaswell modified the milestones: v3.7.0, v3.8.0 Dec 15, 2022
@timhoffm
Copy link
Member Author
timhoffm commented Aug 4, 2023

I've undrafted the original proposal

Note that if you call sharex=True and sharey=True, you do not need to call ax.label_outer to get the current behaviour. This almost says to me that we need a standalone method ax.tick_outer() that does what this kwarg is doing.

Having thought about it: A second method tick_outer() is not the right way forward. I think it's extremely rare, that you'd want to only have outer ticks but keep inner labels unchanged. - IMHO you don't want labels without ticks (or if you want for stylistic reasons, you want them nowhere). In that sense, there's a hirarchy: You always want to specify where your labels are and then optionally have the ticks also only there or not. So extending label_outer() is reasonable.

And yes, while sharing axes already gets us the outer labelling, it's still ok to call label_outer(remove_inner_ticks=True).

One could imagine more fundametal label and tick configuration layouts, but that'd be a bigger cake. Given the simplicity of this extension, I don't have any concerns complicating API or reducing maintainability with the suggested change.

You could also imagine subplots and friends getting a tick_outer kwarg.

That's going too far. subplots has no concern in customizing formatting. The structural configuration share_x, share_y. As a side-effect these can set some formatting defaults (like removing some labels). But we do not want to introduce options. If a user wants different formatting, that's something to be configured afterwards.

@timhoffm timhoffm marked this pull request as ready for review August 4, 2023 14:10
@jklymak
Copy link
Member
jklymak commented Aug 4, 2023

I think vanishingly few people use label_outer - it's not that useful without sharex/y and sharex/y, which basically do what label_outer does. I think this new feature, which is genuinely useful, is going to be hard to find.

jklymak
jklymak reviewed Aug 4, 2023
View reviewed changes
doc/users/next_whats_new/label_outer.rst
``label_outer(remove_inner_ticks=True)``.


.. plot::
Copy link
Member

Choose a reason for hiding this comment

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

This probably needs an example in the docs somewhere as well? Perhaps also mention at https://matplotlib.org/devdocs/users/explain/axes/axes_ticks.html#styling-ticks-tick-parameters?

Copy link
Member Author

Choose a reason for hiding this comment

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

I plan to use it in #24379

jklymak
jklymak approved these changes Aug 4, 2023
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.

I don't think this is very discoverable, but doesn't hurt anything.

@timhoffm timhoffm closed this Aug 6, 2023
@timhoffm
Copy link
Member Author
timhoffm commented Aug 6, 2023 •
edited
Loading

Edit: Ok found out what's wrong. I messed up the rebase so that the branch was identical to main, and force-pushed that. Apparently, because nothing is to do here, Github then closes the PR and claims that it's been merged. 🙄

Something is very broken here on GitHub. In contrast to what the page is claiming I did not close this PR, and the commit is also not on master.

grafik

@timhoffm
Support removing inner ticks in label_outer()
4dfaca5 
Up to now `label_outer()`, only affects tick labels, but not ticks.
This introduces an optional parameter `remove_inner_ticks`, which
removes the inner ticks if the corresponding tick labels are removed.
@timhoffm timhoffm reopened this Aug 6, 2023
@tacaswell tacaswell merged commit 4a34961 into matplotlib:main Aug 9, 2023
@timhoffm timhoffm deleted the label_outer branch August 9, 2023 05:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tacaswell tacaswell tacaswell left review comments

@story645 story645 story645 left review comments

@jklymak jklymak jklymak approved these changes

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

Successfully merging this pull request may close these issues.
4 participants
@timhoffm @story645 @jklymak @tacaswell
0