Skip to content

refactor: Move VTK utilities from geos-posp to geos-mesh #75

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

Merged

Conversation

paloma-martinez
Copy link
Contributor

@paloma-martinez paloma-martinez commented Apr 4, 2025

Close #69

  • This PR includes a little refactor of geos-mesh subfolders, as the name vtk was causing conflict issues in package import.
  • A future PR will be dedicated to handle the addition of geos-mesh package to ci typing-check.

@paloma-martinez paloma-martinez linked an issue Apr 4, 2025 that may be closed by this pull request
@paloma-martinez paloma-martinez marked this pull request as ready for review May 5, 2025 14:36
@@ -25,11 +25,12 @@ classifiers = [
requires-python = ">=3.10"

dependencies = [
"vtk >= 9.3",
"vtk == 9.3",
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the reason for using specificaly 9.3 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems Paraview 5.13 cannot open correctly VTU files generated by VTK > 9.3 at the moment.
As a lot of GEOS users use Paraview, I think it's better to stay with this version of the package as long as it is not handled...

Copy link
Contributor

@mlemayTTE mlemayTTE left a comment

Choose a reason for hiding this comment

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

Very good job to set unit tests up for all utilities.

Copy link
Contributor

Choose a reason for hiding this comment

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

Utilities from geos.mesh.utils.filters and geos.mesh.utils.helpers may be gathered in thematic modules. For instance, a module may contains all attribute helpers (need to set the name "attribute" or "array") and another module may contains other utilities (including getBounds*, to_vtk_id_list, vtk_iter, extractBlock, mergeBlocks, computeCellCenterCoordinates, extractSurfaceFromElevation).

@alexbenedicto alexbenedicto self-requested a review May 14, 2025 21:12
Comment on lines +32 to +34
if keepPartialAttributes:
fillAllPartialAttributes( input, False )
fillAllPartialAttributes( input, True )
Copy link
Contributor

Choose a reason for hiding this comment

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

What is happening in this part?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are filling both cells (False) and point attributes (True)

from vtkmodules.vtkCommonDataModel import vtkUnstructuredGrid, vtkPolyData, vtkPlane
from vtkmodules.vtkFiltersCore import vtk3DLinearGridPlaneCutter

__doc__ = """ Generic VTK utilities."""
Copy link
Contributor

Choose a reason for hiding this comment

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

You may explain a bit more what type of functions this file (same for other files) contains.

@alexbenedicto alexbenedicto self-requested a review May 21, 2025 16:24
@alexbenedicto alexbenedicto merged commit 50b3241 into main May 21, 2025
39 checks passed
@alexbenedicto alexbenedicto deleted the pmartinez/refactor/MoveMultiblockInspectorAndVtkUtils branch May 21, 2025 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move VTK functions and utilities to geos-mesh package
3 participants