-
Notifications
You must be signed in to change notification settings - Fork 20
feat: move geometry commands to versioned architecture #2234
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
base: main
Are you sure you want to change the base?
Conversation
… feat/geometry_commands_restructure # Conflicts: # src/ansys/geometry/core/designer/geometry_commands.py
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2234 +/- ##
==========================================
+ Coverage 94.50% 94.57% +0.07%
==========================================
Files 142 157 +15
Lines 10585 10883 +298
==========================================
+ Hits 10003 10293 +290
- Misses 582 590 +8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
… feat/geometry_commands_restructure
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 go set by set... I simply reviewed the "beams" part for now and it requires a few fixes. Please address them and I'll move on to the next block. Reviewing this PR will take time
Adding comment to keep track of modules reviewed:
|
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.
Reviewed Assembly Controls, Bodies and Commands (plus an extra Beams comment) -- looking good for now. Let's address these
|
||
self._grpc_client.log.debug(f"Creating beams on {self.id}...") | ||
response = self._commands_stub.CreateBeamSegments(request) | ||
response = self._grpc_client.services.beams.create_beam_segments( |
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.
Just remembered, for beams, remove the unnecessary "protect_grpc" statements on the create_beams wrapper method in this module!
# as for deleting a Body | ||
# | ||
self._commands_stub.DeleteBeam(EntityIdentifier(id=beam_requested.id)) | ||
self._grpc_client.services.beams.delete_beam(beam_id=beam_requested.id) |
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.
Delete protect_grpc from this method
return {} | ||
|
||
@protect_grpc | ||
def split_body(self, **kwargs): # noqa: D102 |
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.
Missing dict type hint
raise NotImplementedError | ||
|
||
@protect_grpc | ||
def split_body(self, **kwargs): # noqa: D102 |
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.
Missing dict type hint on return
---------- | ||
selection : list[Body] | list[Component] | list[Face] | list[Edge] | ||
Selection of the object to rename. | ||
selection : list[Body] | list[Component] | ||
Selection of the objects to rename. |
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.
Interesting... renaming does not apply to Face or Edge objects?
Description
Issue linked
#1817
Checklist
feat: extrude circle to cylinder
)