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

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 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:masterfromanntzer:styleref
Jul 4, 2021

Conversation

anntzer
Copy link
Contributor

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:
oldbox
oldarrow

After:
newbox
newarrow

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

# ========== ============== ==========================
# Class Name Attrs
# ========== ============== ==========================
# Circle ``circle`` pad=0.3
# DArrow ``darrow`` pad=0.3
# LArrow ``larrow`` pad=0.3
# RArrow ``rarrow`` pad=0.3
# Round ``round`` pad=0.3,rounding_size=None
# Round4 ``round4`` pad=0.3,rounding_size=None
# Roundtooth ``roundtooth`` pad=0.3,tooth_size=None
# Sawtooth ``sawtooth`` pad=0.3,tooth_size=None
# Square ``square`` pad=0.3
# ========== ============== ==========================
and
# ========== =============================================
# Name Attrs
# ========== =============================================
# ``-`` None
# ``->`` head_length=0.4,head_width=0.2
# ``-[`` widthB=1.0,lengthB=0.2,angleB=None
# ``|-|`` widthA=1.0,widthB=1.0
# ``-|>`` head_length=0.4,head_width=0.2
# ``<-`` head_length=0.4,head_width=0.2
# ``<->`` head_length=0.4,head_width=0.2
# ``<|-`` head_length=0.4,head_width=0.2
# ``<|-|>`` head_length=0.4,head_width=0.2
# ``fancy`` head_length=0.4,head_width=0.4,tail_width=0.4
# ``simple`` head_length=0.5,head_width=0.5,tail_width=0.2
# ``wedge`` tail_width=0.3,shrink_factor=0.5
# ========== =============================================
which can easily go out of sync and aren't particularly nice in the rendered docs either.

(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

  • Has pytest style unit tests (andpytest passes).
  • IsFlake 8 compliant (runflake8 on changed files to check).
  • New features are documented, with examples if plot related.
  • Documentation is sphinx and numpydoc compliant (the docs shouldbuild without error).
  • Conforms to Matplotlib style conventions (installflake8-docstrings and runflake8 --docstring-convention=all).
  • New features have an entry indoc/users/next_whats_new/ (follow instructions in README.rst there).
  • API changes documented indoc/api/next_api_changes/ (follow instructions in README.rst there).

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"))
Copy link
Member

@story645story645Jun 28, 2021
edited
Loading

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.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Actually theseare the default params, and I'd rather keep using the defaults for the reference docs?

Copy link
Member

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?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

It's proportional to the fontsize by default. I'll let you judge whether gray is better...
test

@story645
Copy link
Member

story645 commentedJun 28, 2021
edited
Loading

I like the idea of the parameters next to the images since it makes it very clearthese aren't defaults what's controlled by parameters.

@timhoffm
Copy link
Member

I like the idea of the parameters next to the images since it makes it very clearthese aren't defaults [...].

🤣 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
distincting parameters are 2) It's not clear what these additional parameters mean.

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.

@timhoffm
Copy link
Member

timhoffm commentedJun 28, 2021
edited
Loading

I think, we'd rather want something like

import inspectimport matplotlib.pyplot as pltimport matplotlib.transforms as mtransformsimport matplotlib.patches as mpatchfrom matplotlib.patches import FancyBboxPatchstyles = mpatch.BoxStyle.get_styles()ax = plt.figure(figsize=(5, len(styles))).add_axes([0, 0, 1, 1])ax.set_axis_off()ax.set(xlim=(0, 1), ylim=(len(styles) + 0.5, -0.5))ax.text(.15, 0, 'boxstyle', fontsize=18, ha="center", va="center")ax.text(.45, 0, 'default parameters', fontsize=18, va="center")for i, (stylename, stylecls) in enumerate(styles.items(), start=1):    ax.text(.15, i, stylename, fontsize=15, color='tab:blue',            ha="center", va="center",            bbox=dict(boxstyle=stylename, fc="w", ec="k"))    ax.text(.45, i, str(inspect.signature(stylecls))[1:-1].replace(', ', '\n'),            va='center')

grafik

Similarly for the arrow style (e.g. move the parameters to the right, highlight the style name in blue, make the target marker grey).

story645 reacted with thumbs up emoji

@anntzer
Copy link
ContributorAuthor

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...

arrow
box

@timhoffm
Copy link
Member

These look good. Some minor improvements:

  • The style labels could be a bit larger
  • left-align the boxstyle parameters
  • Agreed that the arrows should stay as is, e.g.
    image
    Nevertheless the parameters could be on their right (parameters are less important than the style names, and english reading direction is left-to-right).
    If you really want to keep the parameters on the left, still consider to left-align the text. I haven't tried but anticipate that this would look cleaner.

@QuLogic
Copy link
Member

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.)

@anntzer
Copy link
ContributorAuthor

Sure. Perhaps you can document that size somewhere?

@timhoffm
Copy link
Member

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

  • figsize=(8: 43
  • figsize=(9: 16
  • figsize=(1x: 22

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 inWriting examples and tutorials.

@anntzer
Copy link
ContributorAuthor

[Agreed that I should just pick a size and go with it for now.]

@jklymakjklymak marked this pull request as draftJuly 1, 2021 19:53
@anntzeranntzer marked this pull request as ready for reviewJuly 4, 2021 16:38
This will allow later removing the table of values in the annotationtutorial.Also, draw things each in their axes and in axes coordinates, tosimplify the code.
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'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 oftransform=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.

@timhoffmtimhoffm added this to thev3.5.0 milestoneJul 4, 2021
@timhoffmtimhoffm merged commit20b13ff intomatplotlib:masterJul 4, 2021
@anntzeranntzer deleted the styleref branchJuly 4, 2021 21:27
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@story645story645story645 left review comments

@timhoffmtimhoffmtimhoffm approved these changes

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

Successfully merging this pull request may close these issues.

5 participants
@anntzer@story645@timhoffm@QuLogic@jklymak

[8]ページ先頭

©2009-2025 Movatter.jp