Skip to content
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

Add interactive plots in documentation #4938

Merged
merged 110 commits into from
Dec 14, 2023
Merged

Add interactive plots in documentation #4938

merged 110 commits into from
Dec 14, 2023

Conversation

ChristosT
Copy link
Contributor

@ChristosT ChristosT commented Sep 20, 2023 •
edited
Loading

Overview

An attempt to bring back the interactive plots in the documentation of pyvista .

This pull request replaces current "pyvista" scraper with the Dynamic scraper in order to allow for interactive examples in the pyvista documentation.

resolves #4877

Live demo

Details

The pyvista-plot directive will now produce interactive scenes by default.
For cases where this does not work well or a static image is preferred a new option
force_static is introduced. See example here

The gallery extension is also updated so that documentation generated from examples contains also interactive scenes.
For this case we provide a fallback option per document (.py file): just define
PYVISTA_GALLERY_FORCE_STATIC_IN_DOCUMENT = True as a global variable in the script.
More fine-grained control can be given by PYVISTA_GALLERY_FORCE_STATIC = True/False which can be defined in block scope example.

In both cases the variable can be hidden from the generated documentation via sphinx_gallery_start_ignore / sphinx_gallery_start_ignore guards.

I went through all the examples and rest of the documentation and turned back to static where the result was sgnificantly different from the current static version. (I may have missed something of course).

Future work:

  • global theme options do not seem to work (see here) , not sure why . I can look into this in a separate pull request.
  • Volume rendering examples do not work. Talking with other developers from Kitware there is a suspicion that it is related with he SmartVolumeMapper that pyvista uses by default for rendering. I will need to investigate this further. For now I left the static images wherever a volume rendering is performed.
  • labels/text is not working for interactive examples I used static images in this case. From what I understand this is a limitation of vtk-js.
  • specular lighting is not working correctly and the result is overexposed, again I fallback to static images. No idea where the problem lies needs more investigation.

Note that as before to test this locally you need to run a http server python -m http.server in the html directory.

@github-actions github-actions bot added documentation Anything related to the documentation/website bug Uh-oh! Something isn't working as expected. labels Sep 20, 2023
@tkoyama010 tkoyama010 mentioned this pull request Sep 20, 2023
Merged
1 task
@tkoyama010 tkoyama010 marked this pull request as draft September 21, 2023 03:33
@tkoyama010 tkoyama010 changed the title Draft: Interactive plots in documentation Add interactive plots in documentation Sep 21, 2023
tkoyama010
tkoyama010 reviewed Sep 21, 2023
View reviewed changes
doc/source/index.rst Outdated Show resolved Hide resolved
@akaszynski
Copy link
Member

Really appreciate your work here @ChristosT. Ping me if you need me to focus on this for a spell. I'm quite busy with a few other projects, but I can spare some dev time over the weekend if needed.

@ChristosT ChristosT marked this pull request as ready for review October 4, 2023 23:37
@tkoyama010 tkoyama010 closed this Oct 5, 2023
@tkoyama010 tkoyama010 reopened this Oct 5, 2023
ChristosT and others added 13 commits October 5, 2023 10:37
@ChristosT
DynamicScraper: Add support for .png and .gif files
71c2bc5 
Query the plotter and use gif files if they are requested.
Use interactive (i.e.) vtksz files only if a scene exists in the plotter
fallback to static png if it does not.
@ChristosT
plot_directive: Add support for interactive scenes vis vrksz files
ba095f4
@ChristosT
plot_directive: Add option 'force_static'
2f29b5a 
Give the option to force a static image even if an interactive scene exists.
This allows to skip cases where the interactive scene is buggy or not as
expected.
@ChristosT
Replace "pyvista" scraper with DynamicScraper
90914e3
@ChristosT
Use plot-directive instead of jupyter-execute in user-guide/*.rst
ec207ed
@ChristosT
plot-directive instead of jupyter-execute in sources/index.rst
42820b8
@tkoyama010 @ChristosT
Apply suggestions from code review
72931c2
@ChristosT
ViewerDirective: Support figure paths in either source or build direc…
216566f 
…tory.

The gallery extension generates the vtksz files under the source while the rest
of the documentation under build.  The ViewerDirective path replacements are
able to handle both now.
@ChristosT
sphinx_gallery: Add support for forcing static images
2caa4ca 
The sphinx gallery will look for the boolean variable
`PYVISTA_GALLERY_FORCE_STATIC_IN_DOCUMENT` in the parsed code to figure out if
static images should be generated instead of an interactive vtksz file.

Generation of static images can be also controlled at a block-level  defining
"PYVISTA_GALLERY_FORCE_STATIC = True" or "PYVISTA_GALLERY_FORCE_STATIC = False"
within the block.
@ChristosT
docs: use static images in examples that interactive does not look right
a9c27b8
@ChristosT
docs: flying edges examples remove specular
cbbdad3 
It does not work well with interactive scenes and since flying edges is all
about creating a mesh it makes sense to sacrifice specular int his case.
@ChristosT
docs: plot-directive instead of jupyter-execute in getting-started/why
aba50b8
@ChristosT
Update tinypages test for offlineviewer
aab22cf
@ChristosT
Copy link
Contributor Author

@akaszynski @tkoyama010 @banesullivan I would appreciate a review or any comments in general :-) . I still need to figure out the failing tinypages test but I don't expect this to affect significantly this PR.

@tkoyama010
Copy link
Member

Considering this is such a dramatic improvement to our user-facing documentation with minor adjustments to the actual package, I'd like to backport this onto the release/0.43 branch unless anyone thinks that might not be a good idea

I have no problem. The documentation has been corrected in past patch releases like typo fixes, and this can be viewed as one of them. It could be taken as a new feature, but since it is a Sphinx extension, it is not a change to PyVista API.

In any case, let me know when you are ready to release. I will do the next patch release once this PR and #5321 are merged.

ChristosT and others added 2 commits December 12, 2023 19:00
@ChristosT @banesullivan
docs: set port of make serve to something predictable
55fce03 
Co-authored-by: Bane Sullivan <banesullivan@gmail.com>
@ChristosT @banesullivan
tests: make package requirements for tests less strict
640123d 
Follows the current convention where:

- requirements_docs.txt: pin everything because our documentation build tends to be fragile and is only fully executed around releases
- requirements_test.txt: limit dependencies by minor release and let dependabot bump these which we can easily pick up on errors, deprecation warnings, or breakages
- requirements.txt: same as requirements_test.txt but this is just the core dependencies

Co-authored-by: Bane Sullivan <banesullivan@gmail.com>
@tkoyama010
Copy link
Member

github-actions preview

banesullivan
banesullivan previously approved these changes Dec 13, 2023
View reviewed changes
Copy link
Member

@banesullivan banesullivan left a comment

Choose a reason for hiding this comment

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

Giving this my rubber stamp though I'd like to peruse the generated documentation one last time

@github-actions GitHub Actions
Copy link
Contributor

🚀 Deployed on https://657914563a990d3c45e77354--tubular-marzipan-5e01ed.netlify.app

@banesullivan
Copy link
Member

@ChristosT, going to let you handle that merge conflict

@tkoyama010 tkoyama010 mentioned this pull request Dec 13, 2023
Closed
15 tasks
@banesullivan banesullivan enabled auto-merge (squash) December 14, 2023 02:32
@banesullivan banesullivan merged commit b960c22 into pyvista:main Dec 14, 2023
25 checks passed
@banesullivan banesullivan mentioned this pull request Dec 14, 2023
Merged
22 tasks
@ChristosT ChristosT mentioned this pull request Dec 15, 2023
Open
@ChristosT
Copy link
Contributor Author

Thank you for the help and guidance this was a long but fun ride ! As discussed in the past I will create issues for the pending items once the documentation is updated online so that I can point to the faulty items.

@ChristosT ChristosT deleted the fix/interactive-plots branch December 15, 2023 01:36
@banesullivan
Copy link
Member

Thanks for the hard work on this @ChristosT! We're actually having issue deploying this to GitHub pages and will have to change our hosting platform before these changes will be visible

@ChristosT
Copy link
Contributor Author

Thanks ! Just for reference, for demo link I used above I deployed manually on netlify through their cli tool. Not sure if this approach is applicable here though .

@banesullivan
Copy link
Member

Yeah, it looks like netlify might be the best, immediate solution. I'm looking into seeing if I can migrate everything there

@banesullivan banesullivan mentioned this pull request Dec 15, 2023
Open
This was referenced Dec 23, 2023
Open
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tkoyama010 tkoyama010 tkoyama010 approved these changes

@banesullivan banesullivan banesullivan approved these changes

Assignees
No one assigned
Labels
bug Uh-oh! Something isn't working as expected. documentation Anything related to the documentation/website
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Interactive plotters in documentation are now static in dev version