refactor: Move PVMergeBlocksEnhanced plugin and create the VTK filter

[paloma-martinez]

Closes #92

[jafranc]

IMO it may help to raise error and keep the old semantics of mergeBlocks() other than that 👍

[jafranc]

Let's keep parenthesis when doing multiple import other than standard

Suggested change

	from geos.utils.Logger import getLogger, Logger, CountWarningHandler
	from geos.utils.Logger import ( getLogger, Logger, CountWarningHandler )

[jafranc]

Suggested change

	from geos.utils.Logger import getLogger, Logger
	from geos.utils.Logger import ( getLogger, Logger )

[jafranc]

wouldn't it be simpler if the behavior of mergeBlocks(mesh) returned mesh if not a vtkMultiBlockDataSet with warning ?

[jafranc]

Suggested change

	_, mergedBlocks = mergeBlocks( compositeBlock )
	mergedBlocks = vtkUnstructuredGrid()
	_, mergedBlocks = mergeBlocks( compositeBlock )

[jafranc]

would help typecheck (maybe)

[jafranc]

Maybe it would be easier to return the merged mesh if composite or multiblock, the unstructured grid input if unstructured and raise an error if other case occur that you could catch here

[jafranc]

Maybe raise a user define error there to avoid the Tuple return

[alexbenedicto]

I agree with @jafranc here, this function should only return a vtkUnstructuredGrid and raise errors when failure.

[alexbenedicto]

As suggested by @jafranc , the mergeBlocks function needs to be reworked to raise errors which will require error handling in all the scripts using it

[alexbenedicto]

I agree with @jafranc here, this function should only return a vtkUnstructuredGrid and raise errors when failure.

[alexbenedicto]

Following the comments made on mergeBlocks function, a try/except block can be used here to evaluate the success of the mergeBlocks function

[jafranc]

I think is we want to properly propagate the error, we should raise it where it happens mergeBlock and deal with it where it makes most sense and where it can either be recovered from or end cleanly.

In most cases I think it is in the PV plugins, and if used outside of it, we should document the errors raised.

I'll push a commit with a proposal for this case

[jafranc]

These last 4-5 commits are proposal on how to handle errors.

As we are dealing with vtkLogger error that are rather registering callbacks than using handlers, there is quite a workaround throwing everything into a buffer to throw on match (here with vtkExecutive ) but could/should be improved.

This is open to comments and rewrite/extension/removal.

I also create an specific VTKError class that is the first of specific PVPluginError (hopefully) if go this path and chose to raise errors rather deep and catch them in RequestData() or other high level.

[paloma-martinez]

I created a child logger inside the mergeBlocks function to prevent unwanted debug messages to appear when executing the plugin.

[alexbenedicto]

Tested the last version with the child logger, works great.

[alexbenedicto]

Genuinely asking, should we also call the VTKCaptureLog here because we are using the vtkMergeBlocks like done in the else block ?

with VTKCaptureLog() as captured_log:
    filter: vtkMergeBlocks = vtkMergeBlocks()
    filter.SetInputData( inputMesh )
    filter.Update()

    captured_log.seek( 0 )
    captured = captured_log.read().decode()

logger.debug( captured.strip() )

And in a more general view, should we use the VTKCaptureLog anytime we use a vtk filter ?

[jafranc]

There is a short and a long answer, but IMO yes.
I will incorporate it in #144 and I think from there we can decide if we want to inc. it everywhere.

[jafranc]

Great ! Great starting point for Error propagation and handling 👍