Skip to content
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

Use annotate coordinate systems to simplify label_subplots. #25905

Merged
merged 1 commit into from
Dec 10, 2023

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented May 17, 2023

The ability of annotate to specify various coordinates systems simplifies label_subplots, by avoiding the need to explicitly construct a ScaledTranslation (while maintaining exact functionality parity).

PR summary

PR checklist

@anntzer anntzer force-pushed the ls branch 2 times, most recently from fda95f2 to 6a6b401 Compare May 21, 2023 10:41
@anntzer
Copy link
Contributor Author

anntzer commented May 21, 2023

Reordered the parameters; also switched to using "offset fontsize" in one of the two examples (thanks @saranti for pointing this out in #25907 (comment)) as that's the more natural unit here.

Comment on lines 25 to 31
# Put the label
# - at the top left corner (axes fraction (0, 1)),
# - offset half-a-fontsize right and half-a-fontsize down
# (offset fontsize (+0.5, -0.5)),
# i.e. just inside the axes.
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if this would be better in the text above instead of a comment, but unfortunately, the CI-built docs have been garbage-collected now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Re-triggered a CI run.

@anntzer
Copy link
Contributor Author

anntzer commented Dec 9, 2023

Should still be good to go.

@jklymak
Copy link
Member

jklymak commented Dec 9, 2023

I'm fine with this, and feel free to ping for a merge when docs finish.

But, did you consider keeping both in the example? ScaledTranslation is useful for other things, so an intuitive application has some value, even if there is a nicer way to do that application?

@anntzer
Copy link
Contributor Author

anntzer commented Dec 9, 2023

Fair point; I restored the use of ScaledTranslation for the second example (I don't think we need to have both versions for both examples, they are really the same).

Copy link
Member

@jklymak jklymak left a comment

Choose a reason for hiding this comment

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

For some reason pre-commit needs to be appeased.

The ability of annotate to specify various coordinates systems
simplifies label_subplots, by avoiding the need to explicitly construct
a ScaledTranslation (while maintaining exact functionality parity).
@anntzer anntzer merged commit d48b2a8 into matplotlib:main Dec 10, 2023
12 of 14 checks passed
@anntzer anntzer deleted the ls branch December 10, 2023 11:49
@anntzer
Copy link
Contributor Author

anntzer commented Dec 10, 2023

Oops, sorry misclicked :/ and accidentally merged just after pushing the pre-commit fix. @jklymak let me know if there's anything else you think should be fixed.

@QuLogic QuLogic added this to the v3.9.0 milestone Dec 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants