feat: commands for merge and intersect by smereu · Pull Request #1665 · ansys/pyansys-geometry

smereu · 2025-01-20T19:10:47Z

Description

This PR switches the implementation of the boolean operations unite(merge) intersect and subtract to using commands rather than core APIs.
Notes

The behavior of intersect -> Keep Other was flipped (wrong) in the original implementation
Some fringe cases (e.g. merge/intersect/subtract of coincident bodies) fail in the new implementation but work in the previous or viceversa. A number of unit tests had to be updated accordingly. @jonahrb it was great to have such extensive coverage

Issue linked

Please mention the issue number or describe the problem this pull request addresses.

Checklist

[ X] I have tested my changes locally.
[ X] I have added necessary documentation or updated existing documentation.
[ X ] I have followed the coding style guidelines of this project.
[ X] I have added appropriate unit tests.
[ X] I have reviewed my changes before submitting this pull request.
I have linked the issue or issues that are solved to the PR if any.
[X ] I have assigned this PR to myself.
[ x] I have added the minimum version decorator to any new backend method implemented.
[ X] I have made sure that the title of my PR follows Conventional commits style (e.g. feat: extrude circle to cylinder)

Boolean operations on body were executed using the core API. For a number of reasons it is agreed to switch to command based implementation which simplifies treatment of occurrences and fixes some known issues. Note that 1) Combine.Merge operation does not support keeping the other bodies 2) Subtract is invoked from Combine.Intersect passing subtract_from_target = true

remove files accidentally committed

smereu · 2025-01-20T19:12:32Z

@RobPasMue @jonahrb @b-matteo Leaving this PR in draft mode until the API server is updated, but feel free to review and add suggestions

Apply style change from pre-commit and correct undefined variable

…b.com/ansys/pyansys-geometry into feat/commands_for_merge_and_intersect

remove unnecessary files

Remove unnecessary files

RobPasMue · 2025-01-21T08:35:12Z

@smereu - please add yourself as "Assignee" of the PR and modify the PR description according to the guidelines whenever you get the chance. This allows us to properly maintain the repository and easily determine features and implementations

…b.com/ansys/pyansys-geometry into feat/commands_for_merge_and_intersect

fix issues with commands and adjust unit tests

…b.com/ansys/pyansys-geometry into feat/commands_for_merge_and_intersect

RobPasMue

Some comments... I still need to process this PR more in depth. My feeling is that there are some unnecessary operations like the final if not keep_other:... or the update_design_inplace.

remove unused option

remove unused argument

For versions prior to 25.2 we have to call the core API methods for boolean since proto changes made in 25.2 are needed for the command implementation

fix use of version and mismatched arguments

…b.com/ansys/pyansys-geometry into feat/commands_for_merge_and_intersect

codecov · 2025-01-27T21:06:55Z

Codecov Report

Attention: Patch coverage is 89.47368% with 4 lines in your changes missing coverage. Please review.

Project coverage is 91.02%. Comparing base (78bde4c) to head (c2f5c93).
Report is 1 commits behind head on blitz.

Files with missing lines	Patch %	Lines
src/ansys/geometry/core/designer/body.py	89.47%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##            blitz    #1665      +/-   ##
==========================================
- Coverage   91.17%   91.02%   -0.15%     
==========================================
  Files          91       91              
  Lines        7924     7959      +35     
==========================================
+ Hits         7225     7245      +20     
- Misses        699      714      +15

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

RobPasMue

Apart from my last comment - LGTM! Let's get it merged soon =)

remove unnecessary argument

RobPasMue

We should revert these breaking changes - sorry, I overlooked them

smereu and others added 3 commits January 20, 2025 13:05

remove files accidentally committed

a749ef8

remove files accidentally committed

chore: auto fixes from pre-commit hooks

a390b49

github-actions bot added the enhancement New features or code improvements label Jan 20, 2025

smereu changed the title ~~Feat/commands for merge and intersect~~ Feat: commands for merge and intersect Jan 21, 2025

smereu and others added 5 commits January 20, 2025 21:32

Apply style change

c92f32b

Apply style change from pre-commit and correct undefined variable

Merge branch 'feat/commands_for_merge_and_intersect' of https://githu…

131ee1e

…b.com/ansys/pyansys-geometry into feat/commands_for_merge_and_intersect

chore: auto fixes from pre-commit hooks

bdc886d

remove unnecessary files

bbc222d

remove unnecessary files

Remove unnecessary files

15ab836

Remove unnecessary files

smereu changed the title ~~Feat: commands for merge and intersect~~ feat: commands for merge and intersect Jan 21, 2025

chore: adding changelog file 1665.added.md [dependabot-skip]

ab2d0d9

Merge branch 'feat/commands_for_merge_and_intersect' of https://githu…

88d0308

…b.com/ansys/pyansys-geometry into feat/commands_for_merge_and_intersect

smereu self-assigned this Jan 22, 2025

smereu and others added 8 commits January 22, 2025 09:42

Merge branch 'blitz' into feat/commands_for_merge_and_intersect

e3d57a1

fix issues with commands and adjust unit tests

fdfe396

fix issues with commands and adjust unit tests

chore: auto fixes from pre-commit hooks

f2ca764

apply style change

080a3f0

Delete test_bool_with_command.py

59a0866

Merge branch 'feat/commands_for_merge_and_intersect' of https://githu…

ebbc293

…b.com/ansys/pyansys-geometry into feat/commands_for_merge_and_intersect

Update test_design.py

0fa653e

chore: auto fixes from pre-commit hooks

3f1cc28

smereu marked this pull request as ready for review January 23, 2025 00:56

smereu requested a review from a team as a code owner January 23, 2025 00:56

smereu requested review from b-matteo and jonahrb January 23, 2025 00:57

smereu added 2 commits January 23, 2025 09:10

Merge branch 'blitz' into feat/commands_for_merge_and_intersect

89d96a0

Update body.py

7bd3041

smereu and others added 3 commits January 23, 2025 09:37

fix style issues

dcfbd02

Update test_design.py

30b6841

Merge branch 'blitz' into feat/commands_for_merge_and_intersect

649e82a

jonahrb reviewed Jan 23, 2025

View reviewed changes

Comment thread src/ansys/geometry/core/designer/body.py

remove keep_other from unite method

31ee1d3

RobPasMue requested changes Jan 24, 2025

View reviewed changes

Comment thread src/ansys/geometry/core/designer/body.py

Comment thread src/ansys/geometry/core/designer/body.py Outdated

Comment thread tests/integration/test_design.py

Comment thread src/ansys/geometry/core/designer/body.py Outdated

smereu and others added 6 commits January 24, 2025 10:21

Update test_design.py

86ea550

remove unused option

Update body.py

29c21e2

remove unused argument

retain backward compatibility

ded21c9

For versions prior to 25.2 we have to call the core API methods for boolean since proto changes made in 25.2 are needed for the command implementation

Merge branch 'blitz' into feat/commands_for_merge_and_intersect

a2b7aa0

Update body.py

086f6dd

fix use of version and mismatched arguments

Merge branch 'feat/commands_for_merge_and_intersect' of https://githu…

8e723a1

…b.com/ansys/pyansys-geometry into feat/commands_for_merge_and_intersect

smereu and others added 2 commits January 28, 2025 07:41

Merge branch 'blitz' into feat/commands_for_merge_and_intersect

846e8df

chore: auto fixes from pre-commit hooks

b5af28a

RobPasMue approved these changes Jan 28, 2025

View reviewed changes

Comment thread tests/integration/test_design.py Outdated

smereu and others added 2 commits January 28, 2025 10:05

Update test_design.py

1086e2f

remove unnecessary argument

Merge branch 'blitz' into feat/commands_for_merge_and_intersect

c67fc38

RobPasMue reviewed Jan 28, 2025

View reviewed changes

Comment thread src/ansys/geometry/core/designer/body.py Outdated

Comment thread src/ansys/geometry/core/designer/body.py Outdated

Comment thread src/ansys/geometry/core/designer/body.py Outdated

Apply suggestions from code review

77a64f0

RobPasMue reviewed Jan 28, 2025

View reviewed changes

Comment thread src/ansys/geometry/core/designer/body.py Outdated

Update src/ansys/geometry/core/designer/body.py

c2f5c93

RobPasMue approved these changes Jan 28, 2025

View reviewed changes

RobPasMue enabled auto-merge (squash) January 28, 2025 16:18

RobPasMue merged commit 6cda8d6 into blitz Jan 28, 2025

RobPasMue deleted the feat/commands_for_merge_and_intersect branch January 28, 2025 16:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: commands for merge and intersect#1665

feat: commands for merge and intersect#1665
RobPasMue merged 36 commits intoblitzfrom
feat/commands_for_merge_and_intersect

smereu commented Jan 20, 2025 •

edited

Loading

Uh oh!

smereu commented Jan 20, 2025 •

edited

Loading

Uh oh!

RobPasMue commented Jan 21, 2025

Uh oh!

Uh oh!

RobPasMue left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Jan 27, 2025 •

edited

Loading

Uh oh!

RobPasMue left a comment

Uh oh!

Uh oh!

RobPasMue left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

smereu commented Jan 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Issue linked

Checklist

Uh oh!

smereu commented Jan 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RobPasMue commented Jan 21, 2025

Uh oh!

Uh oh!

RobPasMue left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Jan 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

RobPasMue left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

RobPasMue left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

smereu commented Jan 20, 2025 •

edited

Loading

smereu commented Jan 20, 2025 •

edited

Loading

codecov bot commented Jan 27, 2025 •

edited

Loading