Skip to content

MNT: Prevent users from erroneously using legend label API on Axis #28584

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
Sep 30, 2024
Merged

MNT: Prevent users from erroneously using legend label API on Axis #28584

merged 1 commit into from
Sep 30, 2024

Conversation

timhoffm
Copy link
Member

Closes #27971.

For a complete explanation see #27971 (comment)

@timhoffm timhoffm force-pushed the axis-get-label branch 3 times, most recently from f848210 to 2a4710a Compare July 17, 2024 12:39
@timhoffm timhoffm marked this pull request as draft July 17, 2024 13:19
@timhoffm
Copy link
Member Author

Need to fix internal usage of Axis.get_label.

timhoffm
timhoffm commented Sep 20, 2024
View reviewed changes
lib/matplotlib/axis.py Outdated
Comment on lines 1434 to 1437
.. admonition:: Discouraged

This overrides `.Artist.get_label`, which is for legend labels, with a new
semantic. It is recommended to use the attribute `.Axis.label` instead.
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've been defensive here and only discouraged but not deprecated this. It's been working for users and I'm not clear whether a deprecation is worth it. We can always add a deprecation later.

@timhoffm timhoffm marked this pull request as ready for review September 20, 2024 10:26
QuLogic
QuLogic reviewed Sep 21, 2024
View reviewed changes
lib/matplotlib/axis.py
.. admonition:: Discouraged

This overrides `.Artist.get_label`, which is for legend labels, with a new
semantic. It is recommended to use the attribute ``Axis.label`` instead.
Copy link
Member

Choose a reason for hiding this comment

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

Can we get Axis.label documented so this will link?

Copy link
Member Author

Choose a reason for hiding this comment

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

The way attributes are currently documented - Attributes section in the class docstring (https://matplotlib.org/stable/api/axis_api.html#matplotlib.axis.Axis) - they cannot be referenced. I believe we must swich to something like #28825. But before going full scale, we should test it on Axes in #28825. This is far beyond this PR. I would like to get this PR into 3.10, but don't want to push the documentation restructuring for it. For now, I suggest that literal is good enough and we can do attribute linking when there is time.

@timhoffm timhoffm added this to the v3.10.0 milestone Sep 24, 2024
@ksunden ksunden merged commit 537ea7a into matplotlib:main Sep 30, 2024
41 of 43 checks passed
@QuLogic
Copy link
Member

QuLogic commented Oct 1, 2024

It looks like we missed that this broke docs?

/home/circleci/project/lib/mpl_toolkits/mplot3d/axis3d.py:docstring of matplotlib.artist.Axis.set:24: WARNING: py:meth reference target not found: matplotlib.axis.Axis.set_label [ref.meth]
/home/circleci/project/lib/matplotlib/projections/polar.py:docstring of matplotlib.artist.RadialAxis.set:24: WARNING: py:meth reference target not found: matplotlib.axis.Axis.set_label [ref.meth]
/home/circleci/project/lib/matplotlib/projections/polar.py:docstring of matplotlib.artist.ThetaAxis.set:24: WARNING: py:meth reference target not found: matplotlib.axis.Axis.set_label [ref.meth]

timhoffm added a commit to timhoffm/matplotlib that referenced this pull request Oct 1, 2024
@timhoffm
DOC: Fix Axis.set_label reference
22c02a8 
Follow-up to matplotlib#28584.
@timhoffm timhoffm mentioned this pull request Oct 1, 2024
Merged
@timhoffm timhoffm deleted the axis-get-label branch October 1, 2024 07:04
kyracho pushed a commit to kyracho/matplotlib that referenced this pull request Oct 10, 2024
@timhoffm @kyracho
MNT: Prevent users from erroneously using legend label API on Axis (m…
ef38f68 
…atplotlib#28584)

Closes matplotlib#27971.

For a complete explanation see matplotlib#27971 (comment)
kyracho pushed a commit to kyracho/matplotlib that referenced this pull request Oct 10, 2024
@timhoffm @kyracho
DOC: Fix Axis.set_label reference
7bb99c3 
Follow-up to matplotlib#28584.
@timhoffm timhoffm mentioned this pull request Dec 11, 2024
Closed
@mfitz mfitz mentioned this pull request Dec 18, 2024
Closed
3 tasks
@timhoffm timhoffm mentioned this pull request Jan 8, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@QuLogic QuLogic QuLogic approved these changes

@ksunden ksunden ksunden approved these changes

Assignees
No one assigned
Labels
Documentation: examples files in galleries/examples GUI: Qt topic: axes topic: mplot3d topic: ticks axis labels
Projects
None yet
Milestone
v3.10.0
Development

Successfully merging this pull request may close these issues.

[Bug]: ax.xaxis.set_label(...) doesn't set the x-axis label
3 participants
@timhoffm @QuLogic @ksunden