Repository navigation
Implement xtick and ytick rotation modes - #28968
Conversation
92c0a43 to
ee92312
Compare
9356808 to
efc02ca
Compare
03d0f37 to
fe4a1c0
Compare
scottshambaugh
left a comment
There was a problem hiding this comment.
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
| """ | ||
| self.set_fontfamily(fontname) | ||
|
|
||
| def ha_for_angle(self, angle): |
There was a problem hiding this comment.
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.
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.
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 = TrueExample
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
timhoffm
left a comment
There was a problem hiding this comment.
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.
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.
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
timhoffm
left a comment
There was a problem hiding this comment.
This is good, apart from mixing the kw translation bugfix into the new feature.
| 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.
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_modetypes'xtick'and'ytick'for the axis labels.The idea is that these new rotation_modes automatically set the label alignments to match any
rotationin'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