-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Show box/arrowstyle parameters in reference examples. #20538
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
ax.text(.5, .5, | ||
f"{stylename}\n{inspect.signature(stylecls)}", | ||
transform=ax.transAxes, ha="center", va="center", | ||
bbox=dict(boxstyle=stylename, fc="w", ec="k")) |
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.
anyway to make the boxes slightly bigger? especially for the sawtooth and round4, the styling gets a little lost in the smaller size.
Sorry, something went wrong.
All reactions
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.
Actually these are the default params, and I'd rather keep using the defaults for the reference docs?
Sorry, something went wrong.
All reactions
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.
Hmm, makes sense, but ugh so hard to differentiate :/ is it cause size is determined by pad? And then hmm, could changing the color help? maybe an inside gray to make the border pop?
Sorry, something went wrong.
All reactions
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.
Sorry, something went wrong.
All reactions
I like the idea of the parameters next to the images since it makes it very clear |
All reactions
Sorry, something went wrong.
🤣 SCNR, but this is part of my point. I see the desire to provide the parameters, but as of now they are really cluttering the images. 1) It's hard to see what the main We don't have rich-text do we? Otherwise we could use color/font weight/font size to highlight the main parameter compared to the other parameters. |
All reactions
Sorry, something went wrong.
I think, we'd rather want something like
Similarly for the arrow style (e.g. move the parameters to the right, highlight the style name in blue, make the target marker grey). |
All reactions
-
👍 1 reaction
Sorry, something went wrong.
How do the ones below look to you? I think specifically for arrows it's better to keep the arrowstyle on the left (because that's what "<-" means...), and then it seems natural to keep the parameters next to the arrowstyle... |
All reactions
Sorry, something went wrong.
All reactions
Sorry, something went wrong.
In #20546, I'm dropping the 50% scale of the figures in the tutorials, because they are generally too small to be useful. I'm not sure exactly how big the figure ends up here now, but in order to avoid scaling, it would be good if you can make the figure fit the text width of the new theme. That would be about 749.167 pixels, or just under 7.5 inches at 100 DPI (though I wonder if we could tweak the theme to be a rounder number.) |
All reactions
Sorry, something went wrong.
Sure. Perhaps you can document that size somewhere? |
All reactions
Sorry, something went wrong.
Previously, the content width was 919px. Reducing that in the new theme is a broader issue that will cause many figures to be rescaled and thus look blurry. We have
It's worth discussing if we want to increase the width of the theme. Going to 800px would at least save the 43 figures with 8inch width. Once decided, the width should be documented in Writing examples and tutorials. |
All reactions
Sorry, something went wrong.
[Agreed that I should just pick a size and go with it for now.] |
All reactions
Sorry, something went wrong.
This will allow later removing the table of values in the annotation tutorial. Also, draw things each in their axes and in axes coordinates, to simplify the code.
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'm not 100% happy with using a grid of Axes instead of positioning the elements in a single Axes. This makes the example more complicated IMHO. In particular, you need a lot of transform=ax.transAxes
, which is a non-trivial concept.
That is okish, because the main purpose of the example is the overview image, not how you generated it. Conceptually simpler code would still be better, but I'll merge as an improvement. We can always have further changes in a followup OR.
Sorry, something went wrong.
All reactions
Successfully merging this pull request may close these issues.
None yet
This will allow later removing the table of values in the annotation
tutorial.
Also, draw things each in their axes and in axes coordinates, to
simplify the code.
Split out of #20531.
Before:


After:


In #20531 (comment) @timhoffm has argued against the addition of all parameters; I think specifically for the example the effect may be neutral or indeed perhaps slightly negative, but the point is actually to remove the tables at
matplotlib/tutorials/text/annotations.py
Lines 139 to 151 in 60ff0d4
matplotlib/tutorials/text/annotations.py
Lines 241 to 256 in 60ff0d4
(Effectively, we have the problem that the example serves both as a standalone example and to generate a figure for the narrative tutorial, but I don't think it's worth duplicating the code to have two separate versions for the two tasks.)
Thoughts?
PR Summary
PR Checklist
pytest
passes).flake8
on changed files to check).flake8-docstrings
and runflake8 --docstring-convention=all
).doc/users/next_whats_new/
(follow instructions in README.rst there).doc/api/next_api_changes/
(follow instructions in README.rst there).