Skip to content

Conversation

@Justineantoine
Copy link
Contributor

@Justineantoine Justineantoine commented Jul 7, 2025

As vtk is not in the project dependencies, a ModuleNotFoundError on vtkmodules is generated when the utils are imported even if vtk is an optional dependency

The AbstractWindow is now a Protocol and the VtkWindow is in its own vtk_utils which is optionally imported if vtk has been installed.
No breaking change.

@Justineantoine Justineantoine requested a review from jourdain July 7, 2025 08:44
@Justineantoine
Copy link
Contributor Author

@Thibault-Pelletier

@Justineantoine Justineantoine force-pushed the fix_unwanted_vtk_import branch from 5e631f4 to 671901e Compare July 7, 2025 09:58
@Justineantoine Justineantoine marked this pull request as draft July 7, 2025 14:13
@Justineantoine Justineantoine force-pushed the fix_unwanted_vtk_import branch from 671901e to cc24899 Compare July 7, 2025 18:21
@Thibault-Pelletier
Copy link
Contributor

@Justineantoine after Alexy's investigation, crash with VTK 9.5 can be fixed by adding import vtkmodules.vtkRenderingOpenGL2 at the top to have the right factory registration before the test run.

@jourdain Should this fix be a separate commit or do we integrate that in Justine's PR?

@jourdain
Copy link
Collaborator

could be in same pr.. Just maybe a commit in itself for clarity. Change logs and releases rely on commits not pr.

@Justineantoine Justineantoine force-pushed the fix_unwanted_vtk_import branch from cc24899 to dba8237 Compare July 28, 2025 07:56
@Justineantoine Justineantoine marked this pull request as ready for review July 28, 2025 08:02
@finetjul
Copy link
Member

@Justineantoine can it be merged ?

@jourdain
Copy link
Collaborator

Should I merge it?

@Justineantoine Justineantoine merged commit 541cc84 into master Sep 18, 2025
6 checks passed
@Justineantoine Justineantoine deleted the fix_unwanted_vtk_import branch October 14, 2025 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants