make several vtk filters accessible in contour filter #421
Conversation
There was a problem hiding this comment.
Looks great! This was on my list of todos a while back but I forgot to implement the other algorithms like marching cubes which is definitely needed
Just a few minor changes that I would like to see, please.
It might be nice to add an example under the filters category showcasing the differences of these three contouring algorithms (if they have noticeable differences for any of the example datasets that we have)
Thanks for making these contributions!
pyvista/core/filters.py
Outdated
| @@ -663,7 +663,7 @@ def elevation(dataset, low_point=None, high_point=None, scalar_range=None, | |||
|
|
|||
| def contour(dataset, isosurfaces=10, scalars=None, compute_normals=False, | |||
| compute_gradients=False, compute_scalars=True, rng=None, | |||
| preference='point'): | |||
| preference='point', method=None): | |||
There was a problem hiding this comment.
Instead of defaulting to None, let's just default to 'contour'
There was a problem hiding this comment.
Or does another one of these algorithms perform better that we should default to?
There was a problem hiding this comment.
the standard contour default is probably safest though
There was a problem hiding this comment.
For my purposes I didn't see any significant difference between them.
I'll switch to contour then. I generally like None for default arguments, because it allows easy propagation of the default value when extending (i.e. creating anything that wraps this method and also wants to expose method) or scripting. E.g. if I want to use the default value unless some condition is met I can do method=None; if special_case: method=marching_cubes; obj.contour(method=method).
There was a problem hiding this comment.
That's a fair point!
Throughout PyVista's API, we have tons of default arguments set to None but they typically lookup a choice/values from somewhere else or a preference from rcParams if None. I'd like to keep it that way for consistency, but you could still leave the if method is None or method == "contour": statement to catch that case.
| """ | ||
| if method == 'contour': |
There was a problem hiding this comment.
| if method == 'contour': | |
| if method is None or method == 'contour': |
No description provided.