Updated Angles on Bracket arrow styles example to make angles clear #23176 #24145

jacoverster · 2022-10-12T14:55:11Z

PR Summary

PR relates to this issue. Added AngleAnnotation patches to mark the angles and included FancyArrowPatch arrows to show the directions of AngleA and AngleB.

PR Checklist

Tests and Styling

Has pytest style unit tests (and pytest passes).
Is Flake 8 compliant (install flake8-docstrings and run flake8 --docstring-convention=all).

Documentation

New features are documented, with examples if plot related.
New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).
Documentation is sphinx and numpydoc compliant (the docs should build without error).

story645 · 2022-10-12T15:11:20Z

I don't think the what's new should be updated mostly cause I don't think this will be very discoverable there and also cause I think we try to avoided updating the what's new docs.

Can you please make this an example in annotations and then link the API docs for arrows to it? @timhoffm may have a better suggestion but I'm not sure what the policy is in terms of these kinds of figures in the reference docs.

ETA: Thanks so much for making this figure and putting in the PR, I'm way psyched for it's existence even if I'm being a grump on placement.

timhoffm · 2022-10-15T20:18:26Z

I wouldn't have a problem updating "what's new" in this case. This is "only" a visual improvement of an existing plot.

However, I agree that it's not very discoverable in there.

Can you please make this an example in annotations and then link the API docs for arrows to it? @timhoffm may have a better suggestion but I'm not sure what the policy is in terms of these kinds of figures in the reference docs.

Fundamentally, illustrating plots are fine the reference docs, see e.g. the notes section of https://matplotlib.org/devdocs/api/ticker_api.html#matplotlib.ticker.ScalarFormatter. However the example code is quite long and would clutter the docstring. So, placing it in the text/annotations section of the examples and linking in the plot via .. plot:: /path/to/example at https://matplotlib.org/devdocs/api/_as_gen/matplotlib.patches.ArrowStyle.html#matplotlib.patches.ArrowStyle is the best solution here.

jacoverster · 2022-10-17T12:44:46Z

Hi @timhoffm and @story645 I've updated the PR with the proposed changes. I created a new example and linked it to matplotlib.patches.ArrowStyle in the example references. I also referenced the example in the ArrowStyle docstring. When I build the documentation it shows up as an example but it does not pull through to https://matplotlib.org/devdocs/api/_as_gen/matplotlib.patches.ArrowStyle.html#matplotlib.patches.ArrowStyle - I assume I'm missing something here so please assist with that.

jklymak · 2022-10-17T14:09:50Z

You've squashed some unrelated commits in here. An easy way to fix is to copy the two files you wanted to change somewhere else, On your branch do git fetch upstream; git reset --hard upstream/main and then copy your two files back to where they belong and make your commit. Alternately, you can do git restore -s upstream/main pathTo/MyFile for the two files.

jacoverster · 2022-10-17T17:00:43Z

Hi @jklymak, sorry about that wat trying to follow the process to amend the previous commit. Did not notice the other commit.

git commit --amend --no-edit
git push [your-remote-repo] [your-branch] --force-with-lease

jacoverster · 2022-10-17T17:27:28Z

You've squashed some unrelated commits in here. An easy way to fix is to copy the two files you wanted to change somewhere else, On your branch do git fetch upstream; git reset --hard upstream/main and then copy your two files back to where they belong and make your commit. Alternately, you can do git restore -s upstream/main pathTo/MyFile for the two files.

Hi @jklymak, not sure how that commit got entangled with mine. I tried this without avail, received the following:

fatal: 'upstream' does not appear to be a git repository
fatal: Could not read from remote repository.

I went ahead and synced my branch with matplotlib:main from GitHub ("sync fork") and then pulled again but now it looks like some of the changes are already merged, so I'm a bit confused. Not sure what to do now.

Please assist and sorry for the mess.

jklymak · 2022-10-17T17:37:06Z

You need to add a "remote" called upstream before you can do that. Our git guide is quite bad, in my opinion. This one is quite good (and I still think we should just copy it, and stop dragging our feet making a new one): https://docs.xarray.dev/en/stable/contributing.html#getting-started-with-git

jacoverster · 2022-10-17T18:02:46Z

Hi @jklymak, thanks I managed to set the upstream and then I ran:

git fetch upstream
git reset --hard upstream/main

Then made the changes again on my branch and ran:

git commit --amend --no-edit
git push --force-with-lease

I think it should be good now. Please confirm.

jklymak · 2022-10-17T18:19:31Z

Looks better. Congrats!

story645 · 2022-10-19T01:46:45Z

examples/text_labels_and_annotations/angles_on_bracket_arrows.py

+from matplotlib.patches import Arc, FancyArrowPatch
+from matplotlib.transforms import Bbox, IdentityTransform, TransformedBbox
+
+


does importing AngleAnnotation from angle_annotation.py work?

Importing AngleAnnotation works but the plots from that example is also automatically generated in the docs then. Not sure if there is a way to exclude them?

hmm, wonder if the trick there is to factor just the AngleAnnotation out into it's own .py file that gets excluded from the sphinx gallery listings and then use a literal block to show it in the scale invariant tutorial.

story645 · 2022-10-19T02:02:26Z

lib/matplotlib/patches.py

+    Examples
+    --------
+    .. plot:: gallery/text_labels_and_annotations/angles_on_bracket_arrows.py


do we usually call this section examples or is notes to be consistent with the other example?

I noticed "Examples" in some docstrings, and "Notes" in others. Seemed like "Examples" was more prevalent. This change did not add the mini gallery to the docs page. Is this the way to do that?

Sorry I spaced - I think notes or examples is contextual and here this plot is a note about how angles work more than an example of how to use brackets

Also according to our docs, the mini gallery should have been registered as part of the references https://matplotlib.org/devdocs/devel/documenting_mpl.html#references-for-sphinx-gallery

Sorry I spaced - I think notes or examples is contextual and here this plot is a note about how angles work more than an example of how to use brackets

Got it, thanks. Will update tot a Note.

Also according to our docs, the mini gallery should have been registered as part of the references https://matplotlib.org/devdocs/devel/documenting_mpl.html#references-for-sphinx-gallery

Is anything else missing @story645? Any ideas why it is not registered?

examples/text_labels_and_annotations/angles_on_bracket_arrows.py

lib/matplotlib/patches.py

story645 · 2022-10-23T17:29:47Z

So I apologize profusely for doing this now, but I have an idea that might simplify things? Maybe instead of making this a new example (w/ all the annoyances of copying the ArcAnnotation class), this gets folded into the ArcAnnotation example as a second example? And then we title that example something like annotating angles - direction and scale invariance? B/C the direction stuff is what's really useful here as an example, and that the figure makes a good bracket reference is kind of independent...(and yes this then is maybe a tutorial, but that's out of scope here)

You can still keep the link to the figure in the reference using the sphinx gallery reference tags https://sphinx-gallery.github.io/stable/advanced.html#cross-referencing

oscargus · 2022-10-23T17:29:47Z

The failing test doesn't seem related to this PR:

/home/circleci/project/doc/api/prev_api_changes/api_changes_3.6.0/removals.rst:157: WARNING: py:obj reference target not found: SubplotBase.get_subplotspec
/home/circleci/project/doc/api/prev_api_changes/api_changes_3.6.0/removals.rst:158: WARNING: py:obj reference target not found: SubplotBase.set_subplotspec

jacoverster · 2022-10-24T12:06:17Z

So I apologize profusely for doing this now, but I have an idea that might simplify things? Maybe instead of making this a new example (w/ all the annoyances of copying the ArcAnnotation class), this gets folded into the ArcAnnotation example as a second example? And then we title that example something like annotating angles - direction and scale invariance? B/C the direction stuff is what's really useful here as an example, and that the figure makes a good bracket reference is kind of independent...(and yes this then is maybe a tutorial, but that's out of scope here)

You can still keep the link to the figure in the reference using the sphinx gallery reference tags https://sphinx-gallery.github.io/stable/advanced.html#cross-referencing

Good suggestion. Alternatively, we could also leave out AngleAnnotation completely (which basically just provides the angle size in this case) and only add the directional arrow. That's what we are really after from the start right, more clarity on the angle direction?

It would then look like this:

story645 · 2022-10-24T14:33:10Z

Alternatively, we could also leave out AngleAnnotation completely

I'd still like labeling of the angle between bracket and perpendicular, but that can probably be achieved w/ ax.text placed relative to the perpendicular.

Also I really like your suggestion!

jacoverster · 2022-10-24T17:53:40Z

Alternatively, we could also leave out AngleAnnotation completely

I'd still like labeling of the angle between bracket and perpendicular, but that can probably be achieved w/ ax.text placed relative to the perpendicular.

Also I really like your suggestion!

Either one is cool, please let me know what the preferred way is 👌🏻

story645 · 2022-10-24T18:48:44Z

I agree w/ you on leaving out ArcAnnotation, (I really like that it's a simpler way to label angles) but if you're gonna go that route please label the angle with ax.text instead.

jacoverster · 2022-10-25T07:46:01Z

Ok, so I've simplified the example significantly by removing AngleAnnotation and adding ax.Text angles. The plot looks like this now:

In the end it's exactly the same output as with AngleAnnotation, just a lot less code.

story645 · 2022-10-25T17:38:31Z

build failed 'cause of unexpected indent but I'm having trouble finding it in the .py file

/home/circleci/project/doc/gallery/text_labels_and_annotations/angles_on_bracket_arrows.rst:32: ERROR: Unexpected indentation.

eta: probably this line going by the rendered docs

jacoverster · 2022-10-25T19:14:26Z

build failed 'cause of unexpected indent but I'm having trouble finding it in the .py file

/home/circleci/project/doc/gallery/text_labels_and_annotations/angles_on_bracket_arrows.rst:32: ERROR: Unexpected indentation.

eta: probably this line going by the rendered docs

Ah, missed a sneaky space there. Should be fixed now. Sorry about that.

story645 · 2022-10-25T19:52:29Z

Sorry about that

No problem/apology needed/wish it was easier to find the errors in the logs :/

story645

The return after the image is weird here but an artifact of docstring interpolation and therefore out of scope for this PR.

story645 · 2022-10-25T20:17:18Z

Um that first commit needs to be removed before I merge this. Can you try rebasing against main and dropping that first commit?

If you need help let somebody (me) know!

examples/text_labels_and_annotations/angles_on_bracket_arrows.py

…example to make angles clear matplotlib#23176

QuLogic · 2022-10-26T09:24:57Z

Thanks @jacoverster! Congratulations on your first PR to Matplotlib 🎉 We hope to hear from you again.

jacoverster · 2022-10-26T11:03:54Z

Thanks @jacoverster! Congratulations on your first PR to Matplotlib 🎉 We hope to hear from you again.
It was a pleasure. Thanks for the guidance @story645 and @jklymak. Would love to contribute more.

…145-on-v3.6.x Backport PR #24145 on branch v3.6.x (Updated Angles on Bracket arrow styles example to make angles clear #23176)

…atplotlib#23176 (matplotlib#24145) * removed AngleAnnotation from angle_on_bracket_arrow example * Fixes indentation mistake. * rebase to main, remove conflicting commit

jacoverster force-pushed the jaco-doc-update branch from be2a1a3 to 9641fff Compare October 17, 2022 12:41

jacoverster closed this Oct 17, 2022

jacoverster force-pushed the jaco-doc-update branch from 9641fff to f95cdd3 Compare October 17, 2022 16:55

jacoverster reopened this Oct 17, 2022

jacoverster force-pushed the jaco-doc-update branch 2 times, most recently from 1b726d0 to 4a3537b Compare October 17, 2022 17:58

story645 mentioned this pull request Oct 19, 2022

[ENH]: Angle Annotation #24217

Closed

story645 reviewed Oct 19, 2022

View reviewed changes

jacoverster commented Oct 20, 2022

View reviewed changes

examples/text_labels_and_annotations/angles_on_bracket_arrows.py Show resolved Hide resolved

story645 reviewed Oct 21, 2022

View reviewed changes

examples/text_labels_and_annotations/angles_on_bracket_arrows.py Show resolved Hide resolved

lib/matplotlib/patches.py Show resolved Hide resolved

jacoverster force-pushed the jaco-doc-update branch from f7d81ec to 84a3c92 Compare October 21, 2022 06:59

story645 self-requested a review October 23, 2022 17:17

story645 self-assigned this Oct 23, 2022

story645 approved these changes Oct 25, 2022

View reviewed changes

story645 added this to the v3.6.2 milestone Oct 25, 2022

story645 linked an issue Oct 25, 2022 that may be closed by this pull request

[Doc]: Bracket ArrowStyle Angle Unclear #23176

Closed

QuLogic reviewed Oct 25, 2022

View reviewed changes

examples/text_labels_and_annotations/angles_on_bracket_arrows.py Outdated Show resolved Hide resolved

jacoverster added 3 commits October 26, 2022 09:39

removed AngleAnnotation from angle_on_bracket_arrow example

9a1d8ef

Fixes indentation mistake.

c2ad2dd

rebase to main, remove conflicting commit

298da48

jacoverster force-pushed the jaco-doc-update branch from a500e3b to 298da48 Compare October 26, 2022 07:48

QuLogic merged commit f15aeee into matplotlib:main Oct 26, 2022

meeseeksmachine mentioned this pull request Oct 26, 2022

Backport PR #24145 on branch v3.6.x (Updated Angles on Bracket arrow styles example to make angles clear #23176) #24280

Merged

meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Oct 26, 2022

Backport PR matplotlib#24145: Updated Angles on Bracket arrow styles …

be5aadb

…example to make angles clear matplotlib#23176

jacoverster deleted the jaco-doc-update branch October 26, 2022 11:02

story645 added a commit that referenced this pull request Oct 26, 2022

Merge pull request #24280 from meeseeksmachine/auto-backport-of-pr-24…

2c43a0d

…145-on-v3.6.x Backport PR #24145 on branch v3.6.x (Updated Angles on Bracket arrow styles example to make angles clear #23176)

ksunden mentioned this pull request Feb 20, 2023

merge up 3.7.0 #25270

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updated Angles on Bracket arrow styles example to make angles clear #23176 #24145

Updated Angles on Bracket arrow styles example to make angles clear #23176 #24145

jacoverster commented Oct 12, 2022

story645 commented Oct 12, 2022 •

edited

Loading

timhoffm commented Oct 15, 2022

jacoverster commented Oct 17, 2022

jklymak commented Oct 17, 2022

jacoverster commented Oct 17, 2022 •

edited

Loading

jacoverster commented Oct 17, 2022

jklymak commented Oct 17, 2022

jacoverster commented Oct 17, 2022

jklymak commented Oct 17, 2022

story645 Oct 19, 2022

jacoverster Oct 19, 2022 •

edited

Loading

story645 Oct 23, 2022

story645 Oct 19, 2022 •

edited

Loading

jacoverster Oct 19, 2022 •

edited

Loading

story645 Oct 19, 2022

story645 Oct 19, 2022 •

edited

Loading

jacoverster Oct 20, 2022

jacoverster Oct 20, 2022

story645 commented Oct 23, 2022 •

edited

Loading

oscargus commented Oct 23, 2022

jacoverster commented Oct 24, 2022 •

edited

Loading

story645 commented Oct 24, 2022 •

edited

Loading

jacoverster commented Oct 24, 2022

story645 commented Oct 24, 2022 •

edited

Loading

jacoverster commented Oct 25, 2022

story645 commented Oct 25, 2022 •

edited

Loading

jacoverster commented Oct 25, 2022

story645 commented Oct 25, 2022

story645 left a comment

story645 commented Oct 25, 2022 •

edited

Loading

QuLogic commented Oct 26, 2022

jacoverster commented Oct 26, 2022

		from matplotlib.patches import Arc, FancyArrowPatch
		from matplotlib.transforms import Bbox, IdentityTransform, TransformedBbox

Updated Angles on Bracket arrow styles example to make angles clear #23176 #24145

Updated Angles on Bracket arrow styles example to make angles clear #23176 #24145

Conversation

jacoverster commented Oct 12, 2022

PR Summary

PR Checklist

story645 commented Oct 12, 2022 • edited Loading

timhoffm commented Oct 15, 2022

jacoverster commented Oct 17, 2022

jklymak commented Oct 17, 2022

jacoverster commented Oct 17, 2022 • edited Loading

jacoverster commented Oct 17, 2022

jklymak commented Oct 17, 2022

jacoverster commented Oct 17, 2022

jklymak commented Oct 17, 2022

story645 Oct 19, 2022

Choose a reason for hiding this comment

jacoverster Oct 19, 2022 • edited Loading

Choose a reason for hiding this comment

story645 Oct 23, 2022

Choose a reason for hiding this comment

story645 Oct 19, 2022 • edited Loading

Choose a reason for hiding this comment

jacoverster Oct 19, 2022 • edited Loading

Choose a reason for hiding this comment

story645 Oct 19, 2022

Choose a reason for hiding this comment

story645 Oct 19, 2022 • edited Loading

Choose a reason for hiding this comment

jacoverster Oct 20, 2022

Choose a reason for hiding this comment

jacoverster Oct 20, 2022

Choose a reason for hiding this comment

story645 commented Oct 23, 2022 • edited Loading

oscargus commented Oct 23, 2022

jacoverster commented Oct 24, 2022 • edited Loading

story645 commented Oct 24, 2022 • edited Loading

jacoverster commented Oct 24, 2022

story645 commented Oct 24, 2022 • edited Loading

jacoverster commented Oct 25, 2022

story645 commented Oct 25, 2022 • edited Loading

jacoverster commented Oct 25, 2022

story645 commented Oct 25, 2022

story645 left a comment

Choose a reason for hiding this comment

story645 commented Oct 25, 2022 • edited Loading

QuLogic commented Oct 26, 2022

jacoverster commented Oct 26, 2022

story645 commented Oct 12, 2022 •

edited

Loading

jacoverster commented Oct 17, 2022 •

edited

Loading

jacoverster Oct 19, 2022 •

edited

Loading

story645 Oct 19, 2022 •

edited

Loading

jacoverster Oct 19, 2022 •

edited

Loading

story645 Oct 19, 2022 •

edited

Loading

story645 commented Oct 23, 2022 •

edited

Loading

jacoverster commented Oct 24, 2022 •

edited

Loading

story645 commented Oct 24, 2022 •

edited

Loading

story645 commented Oct 24, 2022 •

edited

Loading

story645 commented Oct 25, 2022 •

edited

Loading

story645 commented Oct 25, 2022 •

edited

Loading