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

Fix merge dataset #4414

Merged
merged 6 commits into from
May 14, 2023
Merged

Fix merge dataset #4414

merged 6 commits into from
May 14, 2023

Conversation

akaszynski
Copy link
Member

Resolve #4411 by converting merged UnstructuredGrids back to PolyData back using extract_geometry.

There's a performance penalty, but it's worth it since it handles the edge case where we may have an UnstructuredGrid that contains lines or strips.

@github-actions github-actions bot added the bug Uh-oh! Something isn't working as expected. label May 12, 2023
@codecov
Copy link

codecov bot commented May 12, 2023

Codecov Report

Merging #4414 (9de5ff1) into main (438dd0d) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #4414   +/-   ##
=======================================
  Coverage   95.82%   95.83%           
=======================================
  Files          97       97           
  Lines       20875    20909   +34     
=======================================
+ Hits        20004    20038   +34     
  Misses        871      871           

tkoyama010
tkoyama010 previously approved these changes May 12, 2023
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

@RobPasMue
Copy link
Contributor

Thanks for addressing this @akaszynski!

banesullivan
banesullivan previously approved these changes May 12, 2023
@akaszynski akaszynski dismissed stale reviews from banesullivan and tkoyama010 via c80c8aa May 12, 2023 15:53
@akaszynski
Copy link
Member Author

Updated to retain the old behavior when the inputs do not have lines or strips. This helps us retain the "best of both worlds" by using the faster approach when possible, and falling back on the more robust approach when needed.

@banesullivan
Copy link
Member

@akaszynski, I think you merged #4407 into this by accident

@akaszynski akaszynski added this to the v0.39.1 milestone May 13, 2023
@tkoyama010 tkoyama010 merged commit 58f6fbc into main May 14, 2023
24 checks passed
@tkoyama010 tkoyama010 deleted the fix/dataset_merge branch May 14, 2023 22:18
akaszynski added a commit that referenced this pull request May 16, 2023
* convert back to polydata using extact_geometry

* improved readability

* branch for performance

* add in verts

* revert #4407 into this branch

* Avoided omission of two letters.

---------

Co-authored-by: Tetsuo Koyama <tkoyama010@gmail.com>
@akaszynski akaszynski mentioned this pull request May 16, 2023
3 tasks
akaszynski added a commit that referenced this pull request May 16, 2023
* Add PyHyperbolic3D as an external example (#4420)

* Add PyHyperbolic3D as an external example

* Reduce Gif file size

* Use 128 color

* optimize and scale

---------

Co-authored-by: Alex Kaszynski <akascap@gmail.com>

* Fix merge dataset (#4414)

* convert back to polydata using extact_geometry

* improved readability

* branch for performance

* add in verts

* revert #4407 into this branch

* Avoided omission of two letters.

---------

Co-authored-by: Tetsuo Koyama <tkoyama010@gmail.com>

* Remove SKIP in docstrings (#4419)

* remove SKIPs

* remove the rest of skips

* minor fixes

* Fix active scalars and pass point data in `to_tetrahedra` for `RectilinearGrid` (#4406)

* add failing test

* fix active scalars for cell data

* pass point data, deprecate use of pass_cell_data

* document parameter

---------

Co-authored-by: Alex Kaszynski <akascap@gmail.com>

* Add gif extension to Sphinx-Gallery scraper (#4403)

* Add gif extension

* Reset cache

* Add test for GIFs

* Revert cache regeneration

* Fix test

* use cells for the example

---------

Co-authored-by: Alex Kaszynski <akascap@gmail.com>

* bump version to v0.39.1

---------

Co-authored-by: Tetsuo Koyama <tkoyama010@gmail.com>
Co-authored-by: MatthewFlamm <39341281+MatthewFlamm@users.noreply.github.com>
Co-authored-by: Alex Fernandez <21alex295@gmail.com>
@banesullivan banesullivan mentioned this pull request Jun 30, 2023
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Uh-oh! Something isn't working as expected.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adding CircularArcs together does not provide a line
4 participants