Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

added offset section & restructured annotations tutorial#23606

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
timhoffm merged 1 commit intomatplotlib:mainfromstory645:annotation-tutorial
Oct 30, 2022

Conversation

story645
Copy link
Member

@story645story645 commentedAug 12, 2022
edited
Loading

PR Summary

Updated the annotation tutorial to hopefully clarify how relative positioning of offsets works. attn@ubitux

ETA: Also ended up moving things around to try and create some conceptual groupings.

ETA 2: I have no idea what the capitalization convention is supposed to be

PR Checklist

Tests and Styling

  • IsFlake 8 compliant (installflake8-docstrings and runflake8 --docstring-convention=all).

Documentation

  • Documentation is sphinx and numpydoc compliant (the docs shouldbuild without error).

@story645story645 changed the titleadded offset to annotations tutorialadded offset section to annotations tutorialAug 12, 2022
@story645story645 changed the titleadded offset section to annotations tutorialadded offset section & restructered annotations tutorialAug 15, 2022
@story645story645 changed the titleadded offset section & restructered annotations tutorialadded offset section & restructured annotations tutorialAug 15, 2022
@story645story645force-pushed theannotation-tutorial branch 4 times, most recently from286d738 toee8b44eCompareAugust 15, 2022 16:16
@story645story645force-pushed theannotation-tutorial branch 2 times, most recently fromdb4afbb toe5f8bd4CompareAugust 16, 2022 00:51
@madphysicist
Copy link
Contributor

General comment, not sure if it's in scope here: the example figures seem to be unrelated to the code snippets in some cases. That's a bit confusing/distracting. Would you be willing to update?

@story645
Copy link
MemberAuthor

Would you be willing to update?

Absolutely if you're willing to flag 'em.

@story645
Copy link
MemberAuthor

story645 commentedSep 11, 2022
edited
Loading

@timhoffm ,@jklymak ,@QuLogic is it better than what was there enough yet?

@madphysicist can you open an issue about which figures don't match which code? Or if not, let me know and I'll do it. I wanted to do it here but honestly lately this tutorial seems to be getting touched a lot and I don't know how many more times I want to rebase. 🙃

Copy link
Member

@QuLogicQuLogic left a comment

Choose a reason for hiding this comment

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

LGTM.

I didn't realize you moved the custom box style section, but might as well fix it up now.

@madphysicist
Copy link
Contributor

@story645 Looks like the only place on this page with a discrepancy is theConnectionPatch section towards the end. What I did not realize at the time is that you can click on a figure to get the code to generate it, so you can probably just ignore the original comment.

@story645
Copy link
MemberAuthor

@madphysicist I thought I fixed that already by changing the coordinates to

xy = (0.3, 0.2)

can you please double check?

@story645
Copy link
MemberAuthor

story645 commentedSep 25, 2022
edited
Loading

The more I touch this the more I think we should either inset or scrap the linking to the gallery examples 'cause the lack of mechanism to keep them in sync is causing major drift and like parts of this thing read like they refer to gallery examples that no longer exist. Also very out of scope of this PR.

@story645story645 marked this pull request as draftSeptember 25, 2022 19:23
@timhoffm
Copy link
Member

I agree, we should not be relying on figures from examples in the tutorials. The tutorials should be self-contained. It's simple enough to make the plots in there (possibly enhanced by optional foldingsphinx-gallery/sphinx-gallery#953 in the future for plots whose code is not so important). It's ok however, to link to an example in a "see also" style.

story645 reacted with thumbs up emoji

@story645story645 marked this pull request as ready for reviewOctober 16, 2022 08:26
@story645
Copy link
MemberAuthor

So I decided embedding the examples was in scope, I think mostly cause that was easier than checking for/reconciling discrepancies and there were sections of the tutorial that benefited from having the code written out rather than just linking out to a figure.

I intentionally avoided removing stuff from this tutorial, but what belongs in here is definitely arguable. There's now some repetition of gallery examples, but I think that's okay 'cause this allows for divergence down the line. Also a follow up issue/PR would be to clean uphttps://matplotlib.org/devdocs/gallery/userdemo/index.html and maybe to bring in the zoom code if code folding gets implement.

@story645
Copy link
MemberAuthor

Another follow up option is maybe switch out some of the remaining source/target figures for the plot directive + show source link when#24215 goes in

@story645
Copy link
MemberAuthor

Ping: I think it qualifies as better than it was if only cause the code examples now line up w/ the figures and the text.

AnchoredOffsetbox section, embedded code that used to link out togallery examples, also minor consistency and linking tweaks throughoutCo-authored-by: Jody Klymak <jklymak@gmail.com>Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com>Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>Co-authored-by: Joseph Fox-Rabinovitz <madphysicist@users.noreply.github.com>
Copy link
Member

@timhoffmtimhoffm left a comment

Choose a reason for hiding this comment

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

I'll merge, because this is better than the current state. There's still room for improvements, but that can be in followup PRs.

@timhoffmtimhoffm merged commit799f735 intomatplotlib:mainOct 30, 2022
@story645
Copy link
MemberAuthor

Thank you! Totally agree that this needs more work, I just wanted it off my plate.

timhoffm reacted with thumbs up emoji

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@QuLogicQuLogicQuLogic left review comments

@jklymakjklymakjklymak left review comments

@timhoffmtimhoffmtimhoffm approved these changes

@madphysicistmadphysicistmadphysicist approved these changes

Assignees
No one assigned
Projects
None yet
Milestone
v3.7.0
Development

Successfully merging this pull request may close these issues.

5 participants
@story645@madphysicist@timhoffm@QuLogic@jklymak

[8]ページ先頭

©2009-2025 Movatter.jp