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.

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

Use imageio intersphinx links #2489

Merged
merged 2 commits into from
Apr 28, 2022

Conversation

adeak
Copy link
Member

@adeak adeak commented Apr 19, 2022

We have the imageio intersphinx inventory located in doc/conf.py, but we weren't actually using it. This is a quick fix for this.

However, there's a related can of worms here: the links are kind of stale now, because

  1. there's a v2 to v3 API migration
  2. get_writer is no longer documented online
  3. its docstring doesn't actually say much about kwargs which are plugin-specific.

Any recommendation of what we should do on our side? Upgrade our pointers to v3? Can we use a better link for explaining gif/mp4 writer kwargs?

@adeak adeak added documentation Anything related to the documentation/website maintenance Low-impact maintenance activity labels Apr 19, 2022
@codecov
Copy link

codecov bot commented Apr 19, 2022

Codecov Report

Merging #2489 (52f316d) into main (6bb517c) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #2489   +/-   ##
=======================================
  Coverage   93.70%   93.70%           
=======================================
  Files          75       75           
  Lines       16072    16072           
=======================================
  Hits        15061    15061           
  Misses       1011     1011           

@MatthewFlamm
Copy link
Contributor

Is this the documentation for get_writer here:

https://imageio.readthedocs.io/en/stable/reference/userapi.html#imageio.v2.get_writer

I'm not too familiar with how it chooses which writer, but assuming it is ffmpeg, the documented kwargs are here

https://imageio.readthedocs.io/en/stable/_autosummary/imageio.plugins.ffmpeg.html

@adeak
Copy link
Member Author

adeak commented Apr 20, 2022

Is this the documentation for get_writer here:

https://imageio.readthedocs.io/en/stable/reference/userapi.html#imageio.v2.get_writer

Thanks @MatthewFlamm, I forgot that I found that on the page eventually... I could now fix our links by using imageio.v2.get_writer in the link but printing it as imageio.get_writer().

I'm not too familiar with how it chooses which writer, but assuming it is ffmpeg, the documented kwargs are here

https://imageio.readthedocs.io/en/stable/_autosummary/imageio.plugins.ffmpeg.html

I don't know it either. And we also have open_gif which might use another backend. That's kind of my point: I'm wondering if we could get a better pointer for users. Something like "Do reader = imageio.get_writer(filename); help(reader)" (if that works!) might also be an option...

@adeak
Copy link
Member Author

adeak commented Apr 26, 2022

I won't have bandwidth to figure out something better for this in the near future, so we should consider these changes for merging.

Copy link
Contributor

@MatthewFlamm MatthewFlamm 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 okay with the changes. I agree that it would be great to figure out how to point users to the exact kwargs to use, but it doesn't seem straightforward.

@akaszynski akaszynski merged commit b6ab604 into pyvista:main Apr 28, 2022
adeak added a commit to adeak/pyvista that referenced this pull request Apr 28, 2022
* upstream/main:
  Make VTK version error clear when PointSet is still abstract (pyvista#2483)
  Use imageio intersphinx links (pyvista#2489)
  Fix glyphs when orienting with cell data (pyvista#2500)
  Bump mypy from 0.942 to 0.950 (pyvista#2522)
  Update hypothesis requirement from <6.45.1 to <6.45.2 (pyvista#2523)
  Add many Readers (pyvista#2496)
  Bump trimesh from 3.10.8 to 3.11.2 (pyvista#2519)
  Return actor from add_mesh_threshold (pyvista#2516)
  fix uniformgrid.x docstring (pyvista#2511)
  Update imageio requirement from <2.18.0 to <2.19.0 (pyvista#2506)
  Update hypothesis requirement from <6.44.1 to <6.45.1 (pyvista#2507)
  add polyhedral example (pyvista#2505)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Anything related to the documentation/website maintenance Low-impact maintenance activity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants