-
Notifications
You must be signed in to change notification settings - Fork 22
feat: enable get/set persistent ids for stride import/export #1671
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
Conversation
…/stride_import_id
| @@ -0,0 +1,268 @@ | |||
| # Copyright (C) 2023 - 2025 ANSYS, Inc. and/or its affiliates. | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This entire module shouldn't be here... It should go inside the tools subpackage IMO.
RobPasMue
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's hold off merging if possible next time until I have had a look to the PRs. There are some changes requested on my side for this module. Let's handle them when possible @MikeJanes.
Also, the docstrings are not in-line with what the rest of the library is performing. I am talking mostly about the ones inside the unsupported module. Let's revisit them.
The min_backend_version decorator has been overused as well. All private methods "__" shouldn't have them, because you should have checked it in the calling method already (causing an unnecessary reduction in performance).
| # requires a change to core service, uncomment on next core service update | ||
| # assert base_body.id in [b.id for b in bodies1] | ||
| # assert wheel_body.id in [b.id for b in bodies2] | ||
| assert base_body.id not in [b.id for b in bodies1] | ||
| assert wheel_body.id not in [b.id for b in bodies2] | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is going to prevent the update of the Docker images unless we do it right away.... we are checking that the new images are not breaking the tests before promoting them. This is done on main rather than on blitz. And we are safe because of that but nonetheless, it is going to be problematic on the workflows
Description
Add ability to get and set persistent id's on objects during stride import and export
Issue linked
Checklist
feat: extrude circle to cylinder)