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

DOC: fix CL tutorial to give same output from saved file and example#12394

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

Conversation

jklymak
Copy link
Member

@jklymakjklymak commentedOct 3, 2018
edited
Loading

PR Summary

Closes#12391.

When CI passes I'll post the result here for review....

As it'll post in the docs

cl

As it looks in saved file

outname

PR Checklist

  • Has Pytest style unit tests
  • Code isFlake 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

fig.savefig('outname.png', bbox_inches='tight')

#############################################
# A better way to get around this awkwardness is to simply
# usea legendforthe figure:
# usethe legendmethod provided bythe figure class:
Copy link
Member

Choose a reason for hiding this comment

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

simplyuse `.Figure.legend`:

jklymak reacted with thumbs up emoji
@ImportanceOfBeingErnest
Copy link
Member

Yes this works now as expected.
Did you consider point 3. from the list in#12391? (If you think this doesn't apply, that's fine.)

Maybe the text should clearly state here that in order to see the effect one would need to compare the shown (addplt.show() at the end?) with the saved figure? Maybe the linewanttoprint = False can be appended with a comment# set to True and compare saved figure or similar?

@jklymak
Copy link
MemberAuthor

Can we include images in the tutorials? I guess I could just include the png.

WRT item 3 I was t sure it was necessary but happy to do it if you feel it helps

@ImportanceOfBeingErnest
Copy link
Member

I think some of the confusion comes from looking at the produced images, so if at least the code has the savefig command in it, it would point more towards where to look for the feature.

Ithink you can use rst inside the examples, so.. image:: some.png might just work.

@ImportanceOfBeingErnest
Copy link
Member

The manually added figures are a bit... HUGE.

image

But appart this looks good. With both figures shown like this I think the whole section becomes really clear.

@jklymak
Copy link
MemberAuthor

jklymak commentedOct 4, 2018
edited
Loading

@ImportanceOfBeingErnest
Copy link
Member

ImportanceOfBeingErnest commentedOct 5, 2018
edited
Loading

What's the reason for the difference? Shouldn't it save with the same dpi as it shows?

Ah, you changed the width... Why not produce the figure withsavefig(..., dpi=100) locally?

@jklymak
Copy link
MemberAuthor

I had to hard-set the width to 300 px in therst. We are only trying to show what the layout looks like, not what the exact sizing of the png is.

@ImportanceOfBeingErnest
Copy link
Member

image

Copy link
Member

Choose a reason for hiding this comment

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

This makes theLegend section of the constrained layout guide much clearer and it's well suited to prevent misunderstandings as in#12377.

# The saved file looks like:
#
# .. image:: /_static/constrained_layout/CL02.png
# :align: center

Choose a reason for hiding this comment

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

The center alignment seems to be ignored for some reason by the css. I would not consider this crucial at this point. One can revisit the cause of this at a different stage.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

:-( Oh well I tried. As I'm sure you can tell screwing around w/ rst embedded in python comments isn't my favourite pastime.

Choose a reason for hiding this comment

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

There is nothing wrong with the rst command itself. It's either sphinx having changed something in their newest version or us screwing up the CSS.

I think adding something like

img.align-center {        display: block;    margin-left: auto;    margin-right: auto;}

in the css file will center the image correctly.

It's something to keep in mind, but somehow unrelated to this PR anyways.

@jklymak
Copy link
MemberAuthor

jklymak commentedOct 5, 2018
edited
Loading

@ImportanceOfBeingErnestImportanceOfBeingErnest merged commit8ba6651 intomatplotlib:masterOct 5, 2018
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull requestOct 5, 2018
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull requestOct 5, 2018
@jklymakjklymak modified the milestones:v3.0.0-doc,v3.1Oct 5, 2018
@jklymakjklymak deleted the doc-fix-cl-tutorial branchOctober 5, 2018 18:14
@jklymak
Copy link
MemberAuthor

Thanks for the help@ImportanceOfBeingErnest

jklymak added a commit that referenced this pull requestOct 5, 2018
…394-on-v3.0.xBackport PR#12394 on branch v3.0.x (DOC: fix CL tutorial to give same output from saved file and example)
jklymak added a commit that referenced this pull requestOct 5, 2018
…394-on-v3.0.0-docBackport PR#12394 on branch v3.0.0-doc (DOC: fix CL tutorial to give same output from saved file and example)
@QuLogicQuLogic modified the milestones:v3.1,v3.0.0-docOct 6, 2018
@ImportanceOfBeingErnest
Copy link
Member

It seems one drawback of this is that now every time you build the docs it'll create two png images in the tutorials folder, which will then end up in any commit (if one doesn't pay attention).

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

@timhoffmtimhoffmtimhoffm left review comments

@ImportanceOfBeingErnestImportanceOfBeingErnestImportanceOfBeingErnest approved these changes

Assignees
No one assigned
Projects
None yet
Milestone
v3.0.0-doc
Development

Successfully merging this pull request may close these issues.

Constrained Layout tutorial needs some cleanup....
4 participants
@jklymak@ImportanceOfBeingErnest@timhoffm@QuLogic

[8]ページ先頭

©2009-2025 Movatter.jp