-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
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
Implement xtick and ytick rotation modes #28968
Conversation
92c0a43
to
ee92312
Compare
9356808
to
efc02ca
Compare
03d0f37
to
fe4a1c0
Compare
There was a problem hiding this 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:
- Image comparison tests exercising the new codepaths
- A "What's new" entry explaining the new feature
- Updates to some of the examples to show this behavior. Good candidate might be https://matplotlib.org/devdocs/gallery/subplots_axes_and_figures/align_labels_demo.html or https://matplotlib.org/devdocs/gallery/images_contours_and_fields/image_annotated_heatmap.html
lib/matplotlib/text.py
Outdated
@@ -1380,6 +1386,30 @@ def set_fontname(self, fontname): | |||
""" | |||
self.set_fontfamily(fontname) | |||
|
|||
def ha_for_angle(self, angle): |
There was a problem hiding this comment.
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
There was a problem hiding this 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 being covered yet - the angle input here and the va function should be wrapped to [0, 360). In other words: angle = angle % 360
There was a problem hiding this comment.
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()
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!
I didn't actually check the implementation, but does this also work with ticks on the right or the top? |
43d9c02
to
e451e7f
Compare
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.
Thanks! It didn’t handle ticks on the right or top before, but I've updated it to work with those ticks now. |
c074a15
to
ab27d59
Compare
06c5dff
to
249bb58
Compare
There was a problem hiding this 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.
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') |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
@kyracho gentle ping. Do you plan to continue with this? |
fcd99fa
to
ef26713
Compare
8e00cc3
to
1466f04
Compare
There was a problem hiding this 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.
lib/matplotlib/axis.py
Outdated
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) |
There was a problem hiding this comment.
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.
d08f106
to
b670278
Compare
0a30c7e
to
6207976
Compare
09c58e5
to
98778bb
Compare
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! |
Thank you @kyracho ! This is a pretty nice quality of life improvement. |
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.
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.
* 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
Hello, the goal of this PR is to implement two new$[0,360)$ .
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'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:
For top and right labels:
PR checklist