Update extract_values API and add new split_values filter
#6001
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6001 +/- ##
==========================================
+ Coverage 96.94% 96.95% +0.01%
==========================================
Files 140 140
Lines 24413 24494 +81
==========================================
+ Hits 23668 23749 +81
Misses 745 745 |
|
@pyvista-bot preview |
|
LGTM |
tests/core/test_dataset_filters.py
Outdated
|
|
||
| @pytest.mark.parametrize('component_offset', [0, -0.5, None]) | ||
| @pytest.mark.parametrize('component', [0, 1, 'any', 'all']) | ||
| def test_extract_values_component_split(labeled_data, component_offset, component): |
There was a problem hiding this comment.
#6001 (review) is saying this function.
There was a problem hiding this comment.
This test has been updated to remove the if-else blocks. if-else has been replaced with explicit test case tuples which map the inputs to the expected outputs.
user27182
changed the title
component parameter to extract_values filterextract_values API to support multi-component scalars
|
@pyvista-bot preview |
|
@pyvista-bot preview |
user27182
changed the title
extract_values API to support multi-component scalarsextract_values API and add new split_values filter
|
@pyvista-bot preview |
|
@pyvista-bot preview |
There was a problem hiding this comment.
- Remove default behaviour of extracting unique values and splitting the output when no input values are specified.
- ``split=True` must now be set explicitly to split meshes
- Empty input args are no longer accepted
- This default behaviour is what
split_valuesdoes instead.
For the above change, I would like to see a deprecation warning output if a previous input is made. Once that and the minor modifications to the docstring only are fixed, the rest is fine. thank you for making the test code easier to read.
| array_ = array_[:, component_mode_] if num_components > 1 else array_ | ||
| component_logic_function = None | ||
| elif isinstance(component_mode_, str) and component_mode_ in ['any', 'all', 'multi']: | ||
| if array_.ndim == 1: | ||
| component_logic_function = None | ||
| elif component_mode_ == 'any': | ||
| component_logic_function = functools.partial(np.any, axis=1) | ||
| elif component_mode_ in ['all', 'multi']: | ||
| component_logic_function = functools.partial(np.all, axis=1) | ||
| else: | ||
| raise ValueError( | ||
| f"Invalid component '{component_mode_}'. Must be an integer, 'any', 'all', or 'multi'.", | ||
| ) | ||
| return array_, num_components, component_logic_function | ||
|
|
||
| def _get_inputs_from_dict(input_): | ||
| # Get extraction values from dict if applicable. | ||
| # If dict, also validate names/labels mapped to the values | ||
| if not isinstance(input_, dict): | ||
| return None, input_ | ||
| else: | ||
| dict_keys, dict_values = list(input_.keys()), list(input_.values()) | ||
| if all(isinstance(key, str) for key in dict_keys): | ||
| return dict_keys, dict_values | ||
| elif all(isinstance(val, str) for val in dict_values): | ||
| return dict_values, dict_keys | ||
| else: | ||
| raise TypeError( | ||
| "Invalid dict mapping. The dict's keys or values must contain strings.", | ||
| ) | ||
|
|
||
| def _validate_values_and_ranges(array_, values_, ranges_, num_components_, component_mode_): | ||
| # Make sure we have input values to extract | ||
| is_multi_mode = component_mode_ == 'multi' | ||
| if values_ is None: | ||
| if ranges_ is None: | ||
| raise TypeError( | ||
| 'No ranges or values were specified. At least one must be specified.', | ||
| ) | ||
| elif is_multi_mode: | ||
| raise TypeError( | ||
| f"Ranges cannot be extracted using component mode '{component_mode_}'. Expected {None}, got {ranges_}.", | ||
| ) | ||
| elif ( | ||
| len(seq := list(val)) == 2 | ||
| and all(isinstance(item, (int, np.integer, float, np.floating)) for item in seq) | ||
| and seq[0] <= seq[1] | ||
| ): | ||
| values[i] = seq | ||
| continue | ||
| raise ValueError( | ||
| f'Invalid value {val}. Value must be number or a sequence of two numbers representing a [lower, upper] range.', | ||
| ) | ||
| isinstance(values_, str) and values_ == '_unique' | ||
| ): # Private flag used by `split_values` to use unique values | ||
| axis = 0 if is_multi_mode else None | ||
| values_ = np.unique(array_, axis=axis) | ||
|
|
||
| # Validate values | ||
| if values_ is not None: | ||
| if is_multi_mode: | ||
| values_ = np.atleast_2d(values_) | ||
| if values_.ndim > 2: | ||
| raise ValueError( | ||
| f'Component values cannot be more than 2 dimensions. Got {values_.ndim}.', | ||
| ) | ||
| if not values_.shape[1] == num_components_: | ||
| raise ValueError( | ||
| f'Num components in values array ({values_.shape[1]}) must match num components in data array ({num_components_}).', | ||
| ) | ||
| else: | ||
| values_ = np.atleast_1d(values_) | ||
| if values_.ndim > 1: | ||
| raise ValueError( | ||
| f'Values must be one-dimensional. Got {values_.ndim}d values.', | ||
| ) | ||
| if not ( | ||
| np.issubdtype(dtype := values_.dtype, np.floating) | ||
| or np.issubdtype(dtype, np.integer) | ||
| ): | ||
| raise TypeError('Values must be numeric.') | ||
|
|
||
| # Validate ranges | ||
| if ranges_ is not None: | ||
| ranges_ = np.atleast_2d(ranges_) | ||
| if (ndim := ranges_.ndim) > 2: | ||
| raise ValueError(f'Ranges must be 2 dimensional. Got {ndim}.') | ||
| if not ( | ||
| np.issubdtype(dtype := ranges_.dtype, np.floating) | ||
| or np.issubdtype(dtype, np.integer) | ||
| ): | ||
| raise TypeError('Ranges must be numeric.') | ||
| is_valid_range = ranges_[:, 0] <= ranges_[:, 1] | ||
| not_valid = np.invert(is_valid_range) | ||
| if np.any(not_valid): | ||
| invalid_ranges = ranges_[not_valid] | ||
| raise ValueError( | ||
| f'Invalid range {invalid_ranges[0]} specified. Lower value cannot be greater than upper value.', | ||
| ) | ||
| return values_, ranges_ |
There was a problem hiding this comment.
Are we going to move these functions to the validation package?
There was a problem hiding this comment.
Yes we should. I was actually thinking about how much easier it would have been to simply use those instead... I'll work on finishing up #5571 soon.
Thanks, I can add a deprecation warning. In this case a deprecation would probably be needed for other API changes, such as no longer allowing setting a range as part of This filter, |
Oh, I see, that's fine then. No need to do it :) |
|
@tkoyama010 all of your other suggestions (which were marked as resolved) seem reasonable, feel free to apply them all. I didn't see them at first and so only replied to the deprecation comment initially. |
Overview
Follow up from #5963.
In this PR:
extract_valuesto make it more general and more explicitsplit_valuesfilter (direct alternative tosplit_labelsfilter originally proposed in Addsplit_labelsfilter and updatesplit_bodies#5950)These features are motivated by #5968 to enable extracting surfaces generated by the
contour_labelsfilter, which outputs a 2-component scalar array.Details
component_modeparameter with options for extracting values using a single component,'any'component,'all'components, or specific'multi'componentvaluesparameter intovaluesandranges. This makes things more explicit and allows setting a range directly with[lower, upper]instead of requiring[[lower, upper]]valuesnow supports specifying multi-component vectors whencomponent_modeisvalueskeep_original_idsparam intopass_point_idsandpass_cell_idsparameters. This is more consistent with other filters.include_cellsparam dynamic to allow for use with PointSetssplit_valuesdoes instead.