Make VTK version error clear when PointSet is still abstract #2483
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This just missed the patch release I'm putting for #2415. If we need another patch I'll include it. |
Codecov Report
@@ Coverage Diff @@
## main #2483 +/- ##
==========================================
+ Coverage 93.67% 93.72% +0.05%
==========================================
Files 75 75
Lines 16052 16120 +68
==========================================
+ Hits 15036 15108 +72
+ Misses 1016 1012 -4 |
|
Our CI is so hard to impress: |
|
OK, I don't get it. First, why does Second, the second warning with Third, I get a bunch of other warnings and some errors during doc building that we might need to fix: Later some PBR-related warnings that I remember seeing before; not sure if those have always been there or it's a regression, and it might be due to my lack of GPU: VTK errors after the first block of warnings: This is with VTK 9.1.0 with freshly updated |
|
I first saw this when upgrading from This is because we read in boundary conditions by default. |
|
For completeness' sake, looking at https://github.com/pyvista/pyvista/runs/6062869045?check_suite_focus=true it seems that the CGNS VTK warnings and errors are coming from |
akaszynski
approved these changes
Apr 28, 2022
|
Thanks for the commit @akaszynski, I distracted myself by fixing up the validation set on a separate branch, then forgot to come back to this. I'll open that one when this is merged, including this addition. |
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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As of version 0.34.0 we support VTK 9.1's concrete
PointSetclass. However the error we tried to raise in__init__isn't visible when you try to instantiate the class on old VTK:The reason for this is that our
PointSetinherits its__new__method fromvtk.vtkPointSet, which raises this error. The reason why other subclasses work is that they inherint from the corresponding concrete vtk subclasses, e.g.PolyDatafromvtkPolyData.If we move the version check to a newly defined thin
__new__wrapper it seems to work:The test suite seems to pass, so hopefully the wrapping works otherwise.
I don't know if we need a
# pragma: no coverfor the error's branch, I've omitted for now. If codecov screams I'll add it back.