Skip to content

Conversation

@smereu
Copy link
Contributor

@smereu smereu commented Nov 18, 2025

This PR exposes the APIs for enclosure creation. 3 separate APIs are used for box, cylinder and sphere enclosure. Enclosure_options are common to the 3 methods. Other specific arguments are given for each method and corresponding grpc request.

Issue linked

See https://tfs.ansys.com:8443/tfs/ANSYS_Development/Portfolio/_workitems/edit/1369379 request for a zero cushion cylinder enclosure

Checklist

  • I have tested my changes locally.
  • I have added necessary documentation or updated existing documentation.
  • I have followed the coding style guidelines of this project.
  • I have added appropriate unit tests.
  • 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.
  • I have assigned this PR to myself.
  • I have added the minimum version decorator to any new backend method implemented.
  • I have made sure that the title of my PR follows Conventional commits style (e.g. feat: extrude circle to cylinder)

pyansys-ci-bot and others added 20 commits February 17, 2025 14:31
The PR exposes 3 separate APIs for the creation of Box, Cylinder and Sphere enclosure. Enclosure options are common to the 3 cases.  Specific argument are given to each method and grpc request
@smereu smereu self-assigned this Nov 18, 2025
@smereu smereu requested a review from a team as a code owner November 18, 2025 19:36
@github-actions github-actions bot added the enhancement New features or code improvements label Nov 18, 2025
@smereu
Copy link
Contributor Author

smereu commented Nov 18, 2025

@RobPasMue @jacobrkerstetter I put the EnclosureOption class in the Preparetools file, but I can move it to a separate one if we think it is a better pattern

@smereu
Copy link
Contributor Author

smereu commented Nov 19, 2025

@RobPasMue I think it is ready for approval now, minus the fact that there are again two spurious file that don't show up for me in github when I do my commit. Do I need to remove them?

@jacobrkerstetter jacobrkerstetter self-requested a review November 19, 2025 13:58
Copy link
Contributor

@jacobrkerstetter jacobrkerstetter left a comment

Choose a reason for hiding this comment

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

I had one more comment about the EnclosureOptions. Other than that looks great! 😄

Copy link
Member

@RobPasMue RobPasMue left a comment

Choose a reason for hiding this comment

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

Some small comments - once addressed we can merge. Thanks @smereu !

@RobPasMue
Copy link
Member

Do I need to remove them?

No need @smereu - I cleaned up the wrong file for you.. we should really check why you have that file on your side... it's not the first time I see it

smereu and others added 10 commits November 19, 2025 10:21
Co-authored-by: Roberto Pastor Muela <37798125+RobPasMue@users.noreply.github.com>
Co-authored-by: Roberto Pastor Muela <37798125+RobPasMue@users.noreply.github.com>
Co-authored-by: Roberto Pastor Muela <37798125+RobPasMue@users.noreply.github.com>
Co-authored-by: Roberto Pastor Muela <37798125+RobPasMue@users.noreply.github.com>
model used was too new for 24.2 and before 25.1 & 25.2 work fine
@github-actions github-actions bot added the testing Anything related to tests label Nov 19, 2025
Copy link
Member

@RobPasMue RobPasMue left a comment

Choose a reason for hiding this comment

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

LGTM! Ready to merge!

@jacobrkerstetter jacobrkerstetter merged commit a2bdd22 into main Nov 20, 2025
50 checks passed
@jacobrkerstetter jacobrkerstetter deleted the feat/expose_enclosure_methods branch November 20, 2025 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New features or code improvements testing Anything related to tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants