Skip to content

Conversation

@umutsoysalansys
Copy link
Contributor

@umutsoysalansys umutsoysalansys commented Nov 21, 2025

Description

Bodies implementation for V1.

Notes:
Current Test failures on test_design.py

image

Issue linked

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

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)

@github-actions github-actions bot added the enhancement New features or code improvements label Nov 21, 2025
@github-actions github-actions bot added the maintenance Package and maintenance related label Nov 21, 2025
@github-actions github-actions bot removed the maintenance Package and maintenance related label Nov 21, 2025
@codecov
Copy link

codecov bot commented Nov 21, 2025

Codecov Report

❌ Patch coverage is 28.20513% with 28 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.27%. Comparing base (0a694ae) to head (6953bb7).
⚠️ Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
...ys/geometry/core/_grpc/_services/v1/conversions.py 12.50% 28 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2400      +/-   ##
==========================================
- Coverage   91.61%   88.27%   -3.34%     
==========================================
  Files         169      167       -2     
  Lines       11839    11808      -31     
==========================================
- Hits        10846    10424     -422     
- Misses        993     1384     +391     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@RobPasMue RobPasMue mentioned this pull request Nov 21, 2025
23 tasks
@umutsoysalansys umutsoysalansys changed the title feat: bodiesv1 stub chore: bodiesv1 stub Nov 21, 2025
@jacobrkerstetter jacobrkerstetter changed the title chore: bodiesv1 stub chore: v1 implementation of bodies stub Nov 24, 2025
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.

Overall looking really good especially on the bodies.py side. Just a few small changes, nothing major

Comment on lines +1383 to +1388
if isinstance(input, Measurement):
# Measurement object
return GRPCQuantity(value_in_geometry_units=input.value.m_as(DEFAULT_UNITS.SERVER_ANGLE))
else:
# Raw pint Quantity
return GRPCQuantity(value_in_geometry_units=input.m_as(DEFAULT_UNITS.SERVER_ANGLE))
Copy link
Contributor

Choose a reason for hiding this comment

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

same comment here, no pint quantities in. These should be converted before sending to the _services layer

Copy link
Contributor

Choose a reason for hiding this comment

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

like you did above, there is an Angle class

resp = self.stub.RemoveAssignedCADMaterial(request=request)

# Return the response - formatted as a dictionary
return {"successfully_removed": [id for id in resp.successfully_removed_ids]}
Copy link
Contributor

Choose a reason for hiding this comment

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

This might need to be id.id to extract the string

Comment on lines +1017 to +1018
target_selection_ids=[build_grpc_id(target_body.id)],
tool_selection_ids=[build_grpc_id(body.id) for body in other_bodies],
Copy link
Contributor

Choose a reason for hiding this comment

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

We should be sending in just the string so no need for a .id call

Copy link
Contributor

Choose a reason for hiding this comment

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

@umutsoysalansys - still concerned with these. This code in body.py should be updated to send self.id and other.id and then we can remove the .id call on target_body and other_bodies
response = self._template._grpc_client.services.bodies.combine(
target=self,
other=other,
type_bool_op=method,
err_msg=err_msg,
keep_other=keep_other,
transfer_named_selections=False,
)

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.

Definitely want to align the boolean call with how we handle id's before merging but other than that minor comments on conversion. Since I'm doing a cleanup PR later if these need to go in there they can.

Comment on lines +1017 to +1018
target_selection_ids=[build_grpc_id(target_body.id)],
tool_selection_ids=[build_grpc_id(body.id) for body in other_bodies],
Copy link
Contributor

Choose a reason for hiding this comment

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

@umutsoysalansys - still concerned with these. This code in body.py should be updated to send self.id and other.id and then we can remove the .id call on target_body and other_bodies
response = self._template._grpc_client.services.bodies.combine(
target=self,
other=other,
type_bool_op=method,
err_msg=err_msg,
keep_other=keep_other,
transfer_named_selections=False,
)

Comment on lines 1356 to 1364
from ansys.geometry.core.misc.measurements import Distance

# Handle both Measurement objects (which have .value attribute)
if isinstance(input, Distance):
# Measurement object
return GRPCQuantity(value_in_geometry_units=input.value.m_as(DEFAULT_UNITS.SERVER_LENGTH))
else:
# Raw pint Quantity
return GRPCQuantity(value_in_geometry_units=input.m_as(DEFAULT_UNITS.SERVER_LENGTH))
Copy link
Contributor

Choose a reason for hiding this comment

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

since you changed the input type to distance you can probably boil this down to one case

Comment on lines +1383 to +1388
if isinstance(input, Measurement):
# Measurement object
return GRPCQuantity(value_in_geometry_units=input.value.m_as(DEFAULT_UNITS.SERVER_ANGLE))
else:
# Raw pint Quantity
return GRPCQuantity(value_in_geometry_units=input.m_as(DEFAULT_UNITS.SERVER_ANGLE))
Copy link
Contributor

Choose a reason for hiding this comment

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

like you did above, there is an Angle class

return GRPCQuantity(value_in_geometry_units=input.value.m_as(DEFAULT_UNITS.SERVER_LENGTH))
else:
# Raw pint Quantity
return GRPCQuantity(value_in_geometry_units=input.m_as(DEFAULT_UNITS.SERVER_LENGTH))
Copy link
Contributor

Choose a reason for hiding this comment

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

@umutsoysalansys this doesn't seems to work for driving dimension's value, for it doesn't has any "m_as" member.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants