-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
971b5ef
to
9323dee
Compare
286d738
to
ee8b44e
Compare
db4afbb
to
e5f8bd4
Compare
f4a50ba
to
c9468c1
Compare
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? |
Absolutely if you're willing to flag 'em. |
@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. 🙃 |
There was a problem hiding this 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.
@story645 Looks like the only place on this page with a discrepancy is the |
@madphysicist I thought I fixed that already by changing the coordinates to
can you please double check? |
c9468c1
to
bf764c2
Compare
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. |
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 folding sphinx-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. |
c7f11ff
to
a8cde8d
Compare
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 up https://matplotlib.org/devdocs/gallery/userdemo/index.html and maybe to bring in the zoom code if code folding gets implement. |
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 |
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 to gallery examples, also minor consistency and linking tweaks throughout Co-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>
a8cde8d
to
d784332
Compare
There was a problem hiding this 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.
Thank you! Totally agree that this needs more work, I just wanted it off my plate. |
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
flake8-docstrings
and runflake8 --docstring-convention=all
).Documentation