Skip to content

refactor: Add a logger for all the vtk filter of geos-processing and all the plugin of geos-pv#178

Merged
paloma-martinez merged 30 commits intomainfrom
RomainBaville/refactor/AddAndCheckLoggerForFilterAndPlugins
Dec 1, 2025
Merged

refactor: Add a logger for all the vtk filter of geos-processing and all the plugin of geos-pv#178
paloma-martinez merged 30 commits intomainfrom
RomainBaville/refactor/AddAndCheckLoggerForFilterAndPlugins

Conversation

@RomainBaville
Copy link
Copy Markdown
Contributor

@RomainBaville RomainBaville commented Nov 17, 2025

In the context of the code refactoring, this pr aims at creating a logger for all the vtk filters and all the ParaView plugins implemented if absent.

  • Create logger for vtk filters
  • Create logger for ParaView plugins

This pr follows the pr #181.
This pr will not uniformize the output message of these logger. It will be done in another pr following the issue #184

@RomainBaville RomainBaville self-assigned this Nov 17, 2025
@RomainBaville RomainBaville added type: cleanup test-geos-integration Triggers the testing of geosPythonPackages import and integration in GEOS CI type: refactor labels Nov 17, 2025
Romain Baville added 2 commits November 18, 2025 10:40
@RomainBaville RomainBaville marked this pull request as ready for review November 18, 2025 10:07
@RomainBaville RomainBaville added flag: ready for review and removed test-geos-integration Triggers the testing of geosPythonPackages import and integration in GEOS CI labels Nov 18, 2025
@RomainBaville RomainBaville changed the base branch from main to RomainBaville/refactor/RefactorFilterWithoutVTKPythonAlgorytmBase November 20, 2025 10:18
@RomainBaville RomainBaville added DO NOT MERGE! test-geos-integration Triggers the testing of geosPythonPackages import and integration in GEOS CI labels Nov 21, 2025
Copy link
Copy Markdown
Collaborator

@jafranc jafranc left a comment

Choose a reason for hiding this comment

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

I think the try-except block there is some refactoring

Comment on lines +44 to +54


class RaiseMergeBlocks( TestCase ):
"""Test failure on empty multiBlockDataSet."""

def test_TypeError( self ) -> None:
"""Test raise of TypeError."""
multiBlockDataset = vtkMultiBlockDataSet() # should fail on empty data
if Version( vtk.__version__ ) < Version( "9.5" ):
with pytest.raises( VTKError ):
multiblockModifiers.mergeBlocks( multiBlockDataset, True )
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

why in this PR ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

As I changed the filter MergeBlockEnhanced, its test needed to be change to, this test was usless with the new implementation of the filter so I move it to the test of the function mergeBlock

f"The { self.piece } attributes { attributesAlreadyInMeshTo } are already present in the final mesh." )
self.logger.error( f"The filter { self.logger.name } failed." )
return False
try:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I am not sure of try except and raise in the try.
Raising here is fine but capturing should be done in the caller or caller's caller tbf

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

see here

Copy link
Copy Markdown
Contributor Author

@RomainBaville RomainBaville Nov 24, 2025

Choose a reason for hiding this comment

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

In this pr I wanted to add or update the logger. To do that I used the same scheme try/except in all the plugins and filters. But your are right, the filtre may not use try/except. To keep "small" and "unitary" pr, I think it can be done in another pr (see pr #185 )

self.logger.error( f"The new attribute { self.newAttributeName } has not been added." )
self.logger.error( f"The filter { self.logger.name } failed." )
return False
try:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Same here

Romain Baville added 2 commits November 24, 2025 11:40
Base automatically changed from RomainBaville/refactor/RefactorFilterWithoutVTKPythonAlgorytmBase to main November 25, 2025 09:21
Copy link
Copy Markdown
Collaborator

@jafranc jafranc left a comment

Choose a reason for hiding this comment

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

Great then, LF #185

@paloma-martinez paloma-martinez merged commit b5ce4f4 into main Dec 1, 2025
56 checks passed
@paloma-martinez paloma-martinez deleted the RomainBaville/refactor/AddAndCheckLoggerForFilterAndPlugins branch December 1, 2025 10:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

flag: ready for review test-geos-integration Triggers the testing of geosPythonPackages import and integration in GEOS CI type: cleanup type: refactor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants