Skip to content

Implement xtick and ytick rotation modes #28968

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.

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

Merged
merged 1 commit into from
Jan 31, 2025

Conversation

kyracho
Copy link
Contributor

@kyracho kyracho commented Oct 11, 2024

Hello, the goal of this PR is to implement two new rotation_mode types 'xtick' and 'ytick' for the axis labels.
The idea is that these new rotation_modes automatically set the label alignments to match any rotation in $[0,360)$. 'xtick' aligns the x-axis labels, and 'ytick' aligns the y-axis labels.

Closes #28951

Plots illustrating the proposed 'xtick' and 'ytick' rotation modes:

For bottom and left labels:

import matplotlib.pyplot as plt
import numpy as np

t = np.linspace(0, 2 * np.pi, 1000)
r = np.cos(6 * t)
x_flower = r * np.cos(t)
y_flower = r * np.sin(t)

angles, n_cols = np.linspace(0, 360, 25), 5
fig, axs = plt.subplots(nrows=5, ncols=n_cols, figsize=(30, 30))
axs = axs.flatten()
labels = ['label' for i in range(8)]

for i, angle in enumerate(angles):
    ax = axs[i]
    ax.set_title(f'Rotation: {angle:.0f}°', fontsize=10)

    ax.set_xticks(np.linspace(0, 1.3, 8).tolist())
    ax.set_xticklabels(labels)
    # ax.xaxis.tick_top()
    ax.xaxis.set_tick_params(rotation = angle, rotation_mode='xtick')

    ax.set_yticks(np.linspace(0, 1.3, 8).tolist())
    ax.set_yticklabels(labels)
    # ax.yaxis.tick_right()
    ax.yaxis.set_tick_params(rotation = angle, rotation_mode='ytick')

    ax.set_xlim(0, 1.3)
    ax.set_ylim(0, 1.3)
    ax.plot(x_flower * 0.5 + 0.65, y_flower * 0.5 + 0.65, color='purple')


plt.tight_layout()
plt.savefig('rotated_ticks_figure')
plt.show()

rotated_ticks_figure-4

For top and right labels:

import matplotlib.pyplot as plt
import numpy as np

t = np.linspace(0, 2 * np.pi, 1000)
r = np.cos(6 * t)
x_flower = r * np.cos(t)
y_flower = r * np.sin(t)

angles, n_cols = np.linspace(0, 360, 25), 5
fig, axs = plt.subplots(nrows=5, ncols=n_cols, figsize=(30, 30))
axs = axs.flatten()
labels = ['label' for i in range(8)]

for i, angle in enumerate(angles):
    ax = axs[i]
    ax.set_title(f'Rotation: {angle:.0f}°', fontsize=10)

    ax.set_xticks(np.linspace(0, 1.3, 8).tolist())
    ax.set_xticklabels(labels)
    ax.xaxis.tick_top()
    ax.xaxis.set_tick_params(rotation = angle, rotation_mode='xtick')

    ax.set_yticks(np.linspace(0, 1.3, 8).tolist())
    ax.set_yticklabels(labels)
    ax.yaxis.tick_right()
    ax.yaxis.set_tick_params(rotation = angle, rotation_mode='ytick')

    ax.set_xlim(0, 1.3)
    ax.set_ylim(0, 1.3)
    ax.plot(x_flower * 0.5 + 0.65, y_flower * 0.5 + 0.65, color='purple')


plt.tight_layout()
plt.savefig('rotated_ticks_figure')
plt.show()

rotated_ticks_figure-3

PR checklist

@kyracho kyracho force-pushed the positioning_for_rotated_labels branch from 92c0a43 to ee92312 Compare October 11, 2024 11:47
@kyracho kyracho marked this pull request as draft October 11, 2024 12:08
@kyracho kyracho force-pushed the positioning_for_rotated_labels branch 2 times, most recently from 9356808 to efc02ca Compare October 13, 2024 03:01
@kyracho kyracho changed the title Implement xtick rotation mode Implement xtick and ytick rotation modes Oct 13, 2024
@kyracho kyracho force-pushed the positioning_for_rotated_labels branch 3 times, most recently from 03d0f37 to fe4a1c0 Compare October 13, 2024 12:06
Copy link
Contributor

@scottshambaugh scottshambaugh left a comment

Choose a reason for hiding this comment

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

This is looking pretty good I think! I know this is still a draft so apologies if I'm jumping the gun with a review, but please add:

@@ -1380,6 +1386,30 @@ def set_fontname(self, fontname):
"""
self.set_fontfamily(fontname)

def ha_for_angle(self, angle):
Copy link
Contributor

@scottshambaugh scottshambaugh Oct 15, 2024

Choose a reason for hiding this comment

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

angle here and below should be restricted to [0, 360) deg. Docstring should also call out that angle is measured in degrees

Copy link
Contributor

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 being covered yet - the angle input here and the va function should be wrapped to [0, 360). In other words: angle = angle % 360

Copy link
Contributor Author

@kyracho kyracho Oct 21, 2024

Choose a reason for hiding this comment

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

Sorry if I didn't mention this earlier! Matplotlib already applies the modulo operation to the input angle before it reaches _ha_for_angle and _va_for_angle. It applies the modulo operation here:

 def set_rotation(self, s):
        """
        Set the rotation of the text.

        Parameters
        ----------
        s : float or {'vertical', 'horizontal'}
            The rotation angle in degrees in mathematically positive direction
            (counterclockwise). 'horizontal' equals 0, 'vertical' equals 90.
        """
        if isinstance(s, Real):
            self._rotation = float(s) % 360
        elif cbook._str_equal(s, 'horizontal') or s is None:
            self._rotation = 0.
        elif cbook._str_equal(s, 'vertical'):
            self._rotation = 90.
        else:
            raise ValueError("rotation must be 'vertical', 'horizontal' or "
                             f"a number, not {s}")
        self.stale = True

Example

Temporarily adding print statements:

def _ha_for_angle(self, angle, is_tick_top_enabled):
        """
        Determines horizontal alignment ('ha') based on the angle of rotation
        in degrees. Adjusts for is_tick_top_enabled.
        """
        print('angle:', angle)
        if (angle < 5 or 85 <= angle < 105 or 355 <= angle < 360 or
                170 <= angle < 190 or 265 <= angle < 275):
            return 'center'
        elif 5 <= angle < 85 or 190 <= angle < 265:
            return 'left' if is_tick_top_enabled else 'right'
        return 'right' if is_tick_top_enabled else 'left'

    def _va_for_angle(self, angle, is_tick_right_enabled):
        """
        Determines vertical alignment ('va') based on the angle of rotation
        in degrees. Adjusts for is_tick_right_enabled.
        """
        print('angle:', angle)
        if (angle < 5 or 355 <= angle < 360 or 170 <= angle < 190
                or 85 <= angle < 105 or 265 <= angle < 275):
            return 'center'
        elif 190 <= angle < 265 or 5 <= angle < 85:
            return 'baseline' if is_tick_right_enabled else 'top'
        return 'top' if is_tick_right_enabled else 'baseline'

Exercising the code path:

import matplotlib.pyplot as plt
import numpy as np

angles, n_cols = np.linspace(-45, 405, 2), 2
fig, axs = plt.subplots(nrows=1, ncols=n_cols, figsize=(4, 2.5))
axs = axs.flatten()

for i, angle in enumerate(angles):
    ax = axs[i]
    ax.set_title(f'Rotation: {angle:.0f}°', fontsize=10)

    ax.set_xticks([0.5])
    ax.set_xticklabels(['L'])
    ax.xaxis.set_tick_params(rotation=angle, rotation_mode='xtick')

    ax.set_yticks([0.5])
    ax.set_yticklabels(['L'])
    ax.yaxis.set_tick_params(rotation=angle, rotation_mode='ytick')

plt.savefig('rotated_ticks_figure.png')
plt.show()

rotated_ticks_figure

Terminal:

angle: 315.0
angle: 45.0

I restricted the covered angles to [0,360) from [0,360] based on your suggestion. If you still want the modulo operations to be added to _va_for_angle and _ha_for_angle as a precaution, let me know. Thanks!

@anntzer
Copy link
Contributor

anntzer commented Oct 16, 2024

I didn't actually check the implementation, but does this also work with ticks on the right or the top?

@kyracho kyracho force-pushed the positioning_for_rotated_labels branch 2 times, most recently from 43d9c02 to e451e7f Compare October 20, 2024 04:04
@kyracho
Copy link
Contributor Author

kyracho commented Oct 20, 2024

This is looking pretty good I think! I know this is still a draft so apologies if I'm jumping the gun with a review, but please add...

Thanks for the feedback! Added an image comparison test just now. I'll work on the "What's new" entry and the examples once I'm confident that the new changes are OK.

I didn't actually check the implementation, but does this also work with ticks on the right or the top?

Thanks! It didn’t handle ticks on the right or top before, but I've updated it to work with those ticks now.

@kyracho kyracho force-pushed the positioning_for_rotated_labels branch 3 times, most recently from c074a15 to ab27d59 Compare October 20, 2024 04:31
@kyracho kyracho force-pushed the positioning_for_rotated_labels branch from 06c5dff to 249bb58 Compare October 23, 2024 06:33
@github-actions github-actions bot added the Documentation: examples files in galleries/examples label Oct 23, 2024
@kyracho kyracho marked this pull request as ready for review October 23, 2024 06:54
Copy link
Member

@timhoffm timhoffm left a comment

Choose a reason for hiding this comment

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

The fundamental implementation is already correct. The following are merely quality and style improvements.

Comment on lines 21 to 32
for j in range(2):
ax = axs[i][j]
ax.plot(np.arange(1., 0., -0.1) * 1000., np.arange(1., 0., -0.1))
ax.set_title(f'Title {i} {j}')
ax.set_xlabel(f'XLabel {i} {j}')
ax.set_ylabel(f'YLabel {i} {j}')
if (i == 0 and j == 1) or (i == 1 and j == 0):
if i == 0 and j == 1:
ax.xaxis.tick_top()
ax.set_xticks(np.linspace(0, 1000, 5))
ax.set_xticklabels([250 * n for n in range(5)])
ax.xaxis.set_tick_params(rotation=55, rotation_mode='xtick')
Copy link
Member

Choose a reason for hiding this comment

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

I agree that the example is a bit messy and could be improved, but it's largly orthogonal to the tick rotation topic. While one can do small additional improvements along the way, there's a couple of aspects I'd have to comment on.

I suggest to keep the change for this PR to the minimal ax.tick_params(axis='x', rotation=55, rotation_mode='xtick'). If you are interested in improving the example, I'm happy to discuss that separately.

Copy link
Contributor Author

@kyracho kyracho Dec 5, 2024

Choose a reason for hiding this comment

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

TYSM for explaining so clearly, and for your hard work to help me contribute. I really appreciate it! 🩵

A few questions:

There seems to be an issue with

 ax.tick_params(axis='x', rotation=55, rotation_mode='xtick')

It leads to

AttributeError: Line2D.set() got an unexpected keyword argument 'ion_mode'

Tracing to

class Tick(martist.Artist):
                   ⋮
      def __init__(.**kwargs):
                   ⋮
            grid_kw = {k[5:]: v for k, v in kwargs.items()}

The fix I came up with is

grid_kw = {k[5:]: v for k, v in kwargs.items() if k != "rotation_mode"}

to filter out rotation_mode and prevent ion_mode from being passed to the Line2D object.

I'm not sure if it would be better to apply it separately, to the __init__ method, though?

ax.tick_params(axis='x', rotation=55, rotation_mode='xtick')

seems to apply rotation_mode to only the first tick on the axis. Is there a delayed creation of the ticks at render time? so the rotation_mode isn't being applied to all the labels?

I've tried using

ax.set_xticks(ax.get_xticks())
ax.tick_params(axis='x', rotation=55, rotation_mode='xtick')

which seems to create the ticks before setting tick_params, but I'm not sure if it's a good solution?

@timhoffm
Copy link
Member

timhoffm commented Dec 3, 2024

@kyracho gentle ping. Do you plan to continue with this?

@kyracho kyracho force-pushed the positioning_for_rotated_labels branch 6 times, most recently from fcd99fa to ef26713 Compare December 5, 2024 12:56
@kyracho kyracho force-pushed the positioning_for_rotated_labels branch 3 times, most recently from 8e00cc3 to 1466f04 Compare December 5, 2024 13:31
Copy link
Member

@timhoffm timhoffm left a comment

Choose a reason for hiding this comment

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

This is good, apart from mixing the kw translation bugfix into the new feature.

Comment on lines 1098 to 1116
kwtrans = {}
for oldkey, newkey in keymap.items():
if newkey in kw_:
if is_x_axis and newkey == 'label1On':
kwtrans['labelbottom'] = kw_.pop(newkey)
elif is_x_axis and newkey == 'tick1On':
kwtrans['bottom'] = kw_.pop(newkey)
elif is_x_axis and newkey == 'label2On':
kwtrans['labeltop'] = kw_.pop(newkey)
elif is_x_axis and newkey == 'tick2On':
kwtrans['top'] = kw_.pop(newkey)
else:
kwtrans[oldkey] = kw_.pop(newkey)
Copy link
Member

Choose a reason for hiding this comment

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

I'd like to have the bugfix separated from the rotation feature. Either as two commits in this PR, or (slightly perferred) as two separate PRs.

@kyracho kyracho force-pushed the positioning_for_rotated_labels branch 3 times, most recently from d08f106 to b670278 Compare December 7, 2024 03:25
@kyracho kyracho force-pushed the positioning_for_rotated_labels branch from 0a30c7e to 6207976 Compare December 12, 2024 02:20
@kyracho kyracho force-pushed the positioning_for_rotated_labels branch from 09c58e5 to 98778bb Compare December 13, 2024 04:57
@kyracho kyracho requested a review from QuLogic January 8, 2025 23:22
@timhoffm timhoffm added this to the v3.11.0 milestone Jan 18, 2025
@kyracho
Copy link
Contributor Author

kyracho commented Jan 29, 2025

Hi @QuLogic , just checking in on this PR; whenever u have the time, I'd really appreciate your feedback.

Please let me know if there is anything I can improve.

Thank you!

@tacaswell tacaswell merged commit 39c1d50 into matplotlib:main Jan 31, 2025
39 checks passed
@tacaswell
Copy link
Member

Thank you @kyracho ! This is a pretty nice quality of life improvement.

@kyracho kyracho deleted the positioning_for_rotated_labels branch January 31, 2025 20:36
timhoffm added a commit to timhoffm/matplotlib that referenced this pull request Feb 7, 2025
Follow-up to matplotlib#28968.

The rotation_mode was only wired up half way, so that the parameter was
accepted but did not have any effect.
timhoffm added a commit to timhoffm/matplotlib that referenced this pull request Feb 8, 2025
Follow-up to matplotlib#28968.

The rotation_mode was only wired up half way, so that the parameter was
accepted but did not have any effect.
QuLogic pushed a commit that referenced this pull request Feb 11, 2025
* Fix tick_params() label rotation mode

Follow-up to #28968.

The rotation_mode was only wired up half way, so that the parameter was
accepted but did not have any effect.

* Update axis.pyi
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ENH]: Better positioning of rotated tick labels
6 participants