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

RFC: Deploy VTK distribution agnostic PyVista package #6361

Closed
wants to merge 2 commits into from

Conversation

banesullivan
Copy link
Member

@banesullivan banesullivan commented Jul 10, 2024

Request for Comment

We get a number of requests to provide better compatibility between PyVista and some of the different build variants of VTK deployed by Kitware at https://wheels.vtk.org where the distribution name of the package might differ from vtk (e.g., vtk-egl and vtk-osmesa).

Until setuptools solves pypa/setuptools#1139 this might be our best route forward to provide a version of PyVista that end-users can depend on while specifying the exact distribution of VTK they need.

The crux of the problem is that the main pyvista package depends on vtk to make installing PyVista as easy as possible for the average user, but other builds of VTK may have different distribution names like vtk-osmesa and vtk-egl which setuptools/pip cannot properly resolve. Pip sees vtk-osmesa as an entirely seperate package from vtk, because they are - they are different distributions, though they have the same import name of vtk. This leads to namespace collisions if installing both, which is incredibly easy to accidentally do.

In this pull request, I'm proposing that we deploy a second version of pyvista, named pyvista-agnostic pyvista-no-vtk on PyPI that doesn't have a dependency on vtk at all so that end users can specify the exact VTK distribution they need.

This will allow downstream packages to depend on PyVista via the pyvista-no-vtk package and point the the exact package index and distribution name of VTK that they want.

Better naming suggestions welcomed!

What will this look like

Before:

pip install pyvista
pip uninstall vtk
pip install --extra-index-url https://wheels.vtk.org vtk-osmesa

After:

pip install --extra-index-url https://wheels.vtk.org vtk-osmesa pyvista-no-vtk

@pyvista-bot pyvista-bot added the maintenance Low-impact maintenance activity label Jul 10, 2024
@banesullivan
Copy link
Member Author

cc @pyvista/kitware

@banesullivan banesullivan added the deployment Anything related to deploying PyVista to services like PyPI or conda-forge label Jul 10, 2024
Copy link

codecov bot commented Jul 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.23%. Comparing base (1f09b4a) to head (d4e96e7).
Report is 579 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6361   +/-   ##
=======================================
  Coverage   97.23%   97.23%           
=======================================
  Files         142      142           
  Lines       26506    26507    +1     
=======================================
+ Hits        25772    25773    +1     
  Misses        734      734           
🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@akaszynski
Copy link
Member

I've considered this as well as sometimes I have to ship my own custom compiled VTK wheels and pyvista complains when we don't have VTK.

pyvista-no-vtk would be my vote because it's perfectly clear what it's doing. Ideally there would be an option to:

pip install pyvista[no-vtk]

But optional requirements are additive and not subtractive.

@banesullivan
Copy link
Member Author

Ideally there would be an option to pip install pyvista[no-vtk] But optional requirements are additive and not subtractive.

Exactly. pypa/setuptools#1139 would make this work, but until that is implemented upstream, shipping a separate package might be our best bet

@MatthewFlamm
Copy link
Contributor

MatthewFlamm commented Jul 10, 2024

Ideally there would be an option to pip install pyvista[no-vtk] But optional requirements are additive and not subtractive.

Exactly. pypa/setuptools#1139 would make this work, but until that is implemented upstream, shipping a separate package might be our best bet

A relevant thread with a proposal for optional dependency group that is subtractive rather than additive https://discuss.python.org/t/optional-dependency-groups-omitting-package-requirements/20833. This also isn't implemented. (Edited: meant this paragraph to be additive to the above comment, but I didn't word this well enough the first time around)

I'm +1 for the current high level plan and +1 for pyvista-no-vtk.

@adeak
Copy link
Member

adeak commented Jul 10, 2024

It would be nice if we could log some message after installation of the special-cased package, to raise awareness to users installing it by mistake. From a quick search this doesn't seem feasible.

What we can do is modify the agnostic package to check somewhere early if vtk import fails, and if it does, raise an informative error along the lines of "did you forget to install VTK?".

@banesullivan
Copy link
Member Author

With VTK 9.4 this shouldn't be needed. Closing in favor of #6879

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployment Anything related to deploying PyVista to services like PyPI or conda-forge maintenance Low-impact maintenance activity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants