Skip to content

DOC: don't make CL tutorial save new file all the time #17947

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 29, 2020
Merged

DOC: don't make CL tutorial save new file all the time #17947

merged 1 commit into from
Jul 29, 2020

Conversation

jklymak
Copy link
Member

@jklymak jklymak commented Jul 17, 2020 •
edited
Loading

PR Summary

Closes #12677:

Generates the files during build and copies them into _static instead of making them part of the repo. Use .gitignore to ignore local copies (in case some one does git add doc/_static)

PR Checklist

  • Has Pytest style unit tests
  • Code is Flake 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/next_api_changes/* if API changed in a backward-incompatible way

@jklymak jklymak force-pushed the doc-fix-CL-savefile branch from f1cdbb6 to a99cea0 Compare July 17, 2020 02:57
@jklymak
Copy link
Member Author

jklymak commented Jul 17, 2020

This worked fine. Now the file won't be generated each time. OTOH, if we change CL we will have to remember to regenerate it.

@jklymak jklymak marked this pull request as ready for review July 17, 2020 04:07
@jklymak jklymak added this to the 3.2-doc milestone Jul 17, 2020
@QuLogic QuLogic modified the milestones: 3.2-doc, v3.3-doc Jul 17, 2020
@timhoffm
Copy link
Member

Can't you instead simply add the generated files to .gitignore? AFAIK this does not affect the already commited version.

Advantages:

  • It's simpler to update because the files are always generated. Otherwise you'd have to copy the code and run it somewhere.
  • One might even add a script to automatically check if the generated image is still matching the one in the repository.

@jklymak jklymak force-pushed the doc-fix-CL-savefile branch from a99cea0 to 1e87790 Compare July 17, 2020 13:33
@jklymak
Copy link
Member Author

jklymak commented Jul 17, 2020

For some reason the files don't regenerate in the same spot if I just remove them from the repo. Any idea what the new static link should be?

@jklymak
Copy link
Member Author

jklymak commented Jul 17, 2020

i.e. the link /_static/constrained_layout/CL02.png is invalid.

@jklymak jklymak force-pushed the doc-fix-CL-savefile branch 2 times, most recently from 6eae592 to 4c8cf9b Compare July 19, 2020 15:10
@jklymak jklymak force-pushed the doc-fix-CL-savefile branch from 4c8cf9b to 4ada98a Compare July 19, 2020 16:05
@jklymak jklymak requested a review from timhoffm July 19, 2020 17:11
@jklymak
Copy link
Member Author

jklymak commented Jul 19, 2020

Thanks for suggestion @timhoffm - it was a struggle, but I finally got it to work...

timhoffm
timhoffm approved these changes Jul 27, 2020
View reviewed changes
Copy link
Member

@timhoffm timhoffm left a comment

Choose a reason for hiding this comment

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

The relative path ../../doc/_static/constrained_layout_1b.png is not so nice, because the user cannot simply copy the example and run it anywhere. But it's good enough for now.

The elegant solution would be to just save constrained_layout_1b.png wherever it's run, keep the original files in _static/constrained_layout and add a post-processing step to the sphinx run that warns if the static and the generated images differ.

@jklymak
Copy link
Member Author

jklymak commented Jul 27, 2020

Well the sphinx folks said we should write a png "scraper" to copy the file over, and get it included in the written rst. If it is a useful feature elsewhere, I'm happy to implement it. It didn't seem that hard, and basically is what you suggest here...

@timhoffm
Copy link
Member

Working for now. If there is interest in writing a scrapper, that can always be done later.

@timhoffm timhoffm merged commit 57e8fc3 into matplotlib:master Jul 29, 2020
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Jul 29, 2020
@timhoffm @meeseeksmachine
Backport PR matplotlib#17947: DOC: don't make CL tutorial save new fi…
63ddc75 
…le all the time
@meeseeksmachine meeseeksmachine mentioned this pull request Jul 29, 2020
Merged
@jklymak jklymak deleted the doc-fix-CL-savefile branch July 29, 2020 13:41
jklymak added a commit that referenced this pull request Jul 29, 2020
@jklymak
Merge pull request #18116 from meeseeksmachine/auto-backport-of-pr-17…
2cf57ec 
…947-on-v3.3.0-doc

Backport PR #17947 on branch v3.3.0-doc (DOC: don't make CL tutorial save new file all the time)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tacaswell tacaswell tacaswell approved these changes

@timhoffm timhoffm timhoffm approved these changes

Assignees
No one assigned
Labels
Documentation
Projects
None yet
Milestone
v3.3-doc
Development

Successfully merging this pull request may close these issues.

DOC: CL tutorial now saves png each time...
4 participants
@jklymak @timhoffm @tacaswell @QuLogic