FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Fix `clabel` manual argument not accepting unit-typed coordinates by aman-coder03 · Pull Request #31278 · matplotlib/matplotlib · GitHub

/ matplotlib Public

Fix clabel manual argument not accepting unit-typed coordinates - #31278

Merged
story645 merged 4 commits into
matplotlib:mainfrom
aman-coder03:fix/clabel-manual-units
Mar 11, 2026
ConversationCommits (4)ChecksFiles changed
Merged

Fix clabel manual argument not accepting unit-typed coordinates#31278
story645 merged 4 commits into
matplotlib:mainfrom
aman-coder03:fix/clabel-manual-units

Conversation

aman-coder03 commented Mar 11, 2026

Copy link
Copy Markdown
Contributor

PR summary

when unit typed coordinates(e.g datetime) are passed to the manual argument of clabel...they were passed directly to transform.transform() without unit conversion, causing a TypeError because the transform expects float64 but received Python objects

fixed by calling self.axes.convert_xunits(x) and self.axes.convert_yunits(y) in add_label_near() before applying the transform. This is the same pattern already used elsewhere in _base.py and is a no-op when nounit converter is registered, so non-unit axes are unaffected

AI Disclosure

PR checklist

github-actions Bot commented Mar 11, 2026

Copy link
Copy Markdown

Thank you for opening your first PR into Matplotlib!

If you have not heard from us in a week or so, please leave a new comment below and that should bring it to our attention. Most of our reviewers are volunteers and sometimes things fall through the cracks.

You can also join us on gitter for real-time discussion.

For details on testing, writing docs, and our review process, please see the developer guide.
Please let us know if (and how) you use AI, it will help us give you better feedback on your PR.

We strive to be a welcoming and open project. Please follow our Code of Conduct.

aman-coder03 mentioned this pull request Mar 11, 2026
Closed

rcomer commented Mar 11, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Hi @aman-coder03 thank you for your interest in contributing to Matplotlib! Please can you run the example from the issue with your branch, and post the image that it now produces.

aman-coder03 commented Mar 11, 2026

Copy link
Copy Markdown
Contributor Author

sure! here is the output produced by the example from the issue with my fix applied

aman-coder03 commented Mar 11, 2026

Copy link
Copy Markdown
Contributor Author

failing tests are unrelated to this fix test_webagg is a known flaky timeout issue and test_upsample_interpolation_stage is a subprocess resource warning... all other 9921 tests pass

rcomer commented Mar 11, 2026

Copy link
Copy Markdown
Member

Thanks for posting the image. I agree that looks correct. Please can you modify this test so it covers the case you are fixing.

matplotlib/lib/matplotlib/tests/test_datetime.py

Lines 262 to 266 in 51a1ab6

@pytest.mark.xfail(reason="Test for clabel not written yet")
@mpl.style.context("default")
def test_clabel(self):
fig, ax = plt.subplots()
ax.clabel(...)

aman-coder03 commented Mar 11, 2026

Copy link
Copy Markdown
Contributor Author

@rcomer i have added the test now!

rcomer commented Mar 11, 2026

Copy link
Copy Markdown
Member

Thanks for adding the test. Is there any way it could be simplified, but still fail without your change?

aman-coder03 commented Mar 11, 2026

Copy link
Copy Markdown
Contributor Author

yes @rcomer working on simplifying it, will push a commit shortly

melissawm moved this to Needs review in First Time Contributors Mar 11, 2026
rcomer approved these changes Mar 11, 2026
View reviewed changes

rcomer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thanks @aman-coder03. This seems right to me.

story645 reviewed Mar 11, 2026
View reviewed changes
Comment thread lib/matplotlib/contour.py
Comment thread lib/matplotlib/tests/test_datetime.py Outdated
fig, ax = plt.subplots()
ax.clabel(...)
CS = ax.contour(X, Y, Z)
ax.clabel(CS, manual=[(x[0], dates[0])])

story645 Mar 11, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Can you test that the label is set to the expected value?

aman-coder03 Mar 11, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

sure i can add an assertion on the label value, what would be the best way to check it, assert on the text string or the position?

story645 Mar 11, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Both?

aman-coder03 Mar 11, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Done!

story645 modified the milestones: v3.11.0, v3.10.9 Mar 11, 2026
story645 merged commit a01f57d into matplotlib:main Mar 11, 2026
32 of 36 checks passed
github-project-automation Bot moved this from Needs review to Merged in First Time Contributors Mar 11, 2026
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Mar 11, 2026
Backport PR matplotlib#31278: Fix clabel manual argument not accept…
ef19bdc 
…ing unit-typed coordinates
meeseeksmachine mentioned this pull request Mar 11, 2026
Merged
andreas16700 added a commit to andreas16700/matplotlib that referenced this pull request Mar 16, 2026
Run a01f57d: Fix clabel manual argument not accepting unit-typed co…
4726c51 
…ordinates (matplotlib#31278)
andreas16700 added a commit to andreas16700/matplotlib that referenced this pull request Mar 16, 2026
Run a01f57d: Fix clabel manual argument not accepting unit-typed co…
da12b31 
…ordinates (matplotlib#31278)
ksunden added a commit that referenced this pull request Apr 21, 2026
Merge pull request #31280 from meeseeksmachine/auto-backport-of-pr-31…
851f769 
…278-on-v3.10.x

Backport PR #31278 on branch v3.10.x (Fix `clabel` manual argument not accepting unit-typed coordinates)
This was referenced Apr 21, 2026
Closed
Closed
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

story645 story645 approved these changes
rcomer rcomer approved these changes

Assignees

No one assigned

Labels

first-contribution topic: contour

Projects

First Time Contributors
Status: Merged

Milestone

v3.10.9

Development

Successfully merging this pull request may close these issues.

[Bug]: clabel manual argument does not accept units

4 participants

Back | FazBrowse Home | New Git URL