Skip to content
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

Add integrate_data filter #2790

Merged
merged 11 commits into from
Jun 16, 2022
Merged

Add integrate_data filter #2790

merged 11 commits into from
Jun 16, 2022

Conversation

MatthewFlamm
Copy link
Contributor

@MatthewFlamm MatthewFlamm commented Jun 14, 2022

Overview

Closes #2789 . Adds ability to integrate data in a dataset including area or volume.

Details

The vtk class vtkIntegrateAttributes includes a SetDivideAllCellDataByVolume() method, but I have deliberately chosen not to implement it and in fact have explicitly turned it off. When enabled, only the integrated cell data is divided by the area or volume. This could be confusing, and it is straightforward for the user to implement themselves.

The return from this filter is a bit weird at first, an UnstructuredGrid with 1 point and 1 cell. I considered returning a dict, e.g. {"data": 99.1, "Area": 2.45}. But given that we can have name clashes between point and cell data arrays, we'd have to return two dicts. So I have chosen to keep the default vtk behavior, and tried to explain this multiple times in the documentation and examples.

@github-actions github-actions bot added documentation Anything related to the documentation/website enhancement Changes that enhance the library labels Jun 14, 2022
@codecov
Copy link

codecov bot commented Jun 14, 2022

Codecov Report

Merging #2790 (3d06a2f) into main (b4abf24) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #2790   +/-   ##
=======================================
  Coverage   93.91%   93.91%           
=======================================
  Files          76       76           
  Lines       16298    16305    +7     
=======================================
+ Hits        15306    15313    +7     
  Misses        992      992           

@MatthewFlamm
Copy link
Contributor Author

MatthewFlamm commented Jun 14, 2022

I intended to add to the gallery example one that uses a volume, but I ran out of implementation scheme steam. This might be a good 'first issue' that we can add.

Copy link
Member

@tkoyama010 tkoyama010 left a comment

Choose a reason for hiding this comment

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

Thank you for adding this.

pyvista/core/filters/data_set.py Outdated Show resolved Hide resolved
pyvista/core/filters/data_set.py Show resolved Hide resolved
Copy link
Member

@akaszynski akaszynski left a comment

Choose a reason for hiding this comment

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

Thanks for adding this!

Extended this example with a simple volume example.

LGTM.

Copy link
Member

@tkoyama010 tkoyama010 left a comment

Choose a reason for hiding this comment

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

LGTM

@tkoyama010 tkoyama010 merged commit 1d33e66 into main Jun 16, 2022
@tkoyama010 tkoyama010 deleted the feat/add-integrate-data-filter branch June 16, 2022 08:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Anything related to the documentation/website enhancement Changes that enhance the library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Integrate a variable
3 participants