Fix clabel manual argument not accepting unit-typed coordinates#31278
Fix clabel manual argument not accepting unit-typed coordinates#31278story645 merged 4 commits intomatplotlib:mainfrom
clabel manual argument not accepting unit-typed coordinates#31278Conversation
|
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. We strive to be a welcoming and open project. Please follow our Code of Conduct. |
|
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. |
|
sure! here is the output produced by the example from the issue with my fix applied |
|
failing tests are unrelated to this fix |
|
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 |
|
@rcomer i have added the test now! |
|
Thanks for adding the test. Is there any way it could be simplified, but still fail without your change? |
|
yes @rcomer working on simplifying it, will push a commit shortly |
rcomer
left a comment
rcomer
left a comment
There was a problem hiding this comment.
Thanks @aman-coder03. This seems right to me.
| fig, ax = plt.subplots() | ||
| ax.clabel(...) | ||
| CS = ax.contour(X, Y, Z) | ||
| ax.clabel(CS, manual=[(x[0], dates[0])]) |
There was a problem hiding this comment.
Can you test that the label is set to the expected value?
There was a problem hiding this comment.
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?
github-project-automation
bot
moved this from Needs review
to Merged
in First Time Contributors
…ing unit-typed coordinates
4 participants
PR summary
when unit typed coordinates(e.g datetime) are passed to the
manualargument ofclabel...they were passed directly totransform.transform()without unit conversion, causing aTypeErrorbecause the transform expects float64 but received Python objectsfixed by calling
self.axes.convert_xunits(x)andself.axes.convert_yunits(y)inadd_label_near()before applying the transform. This is the same pattern already used elsewhere in_base.pyand is a no-op when nounit converter is registered, so non-unit axes are unaffectedAI Disclosure
PR checklist