-
Notifications
You must be signed in to change notification settings - Fork 128
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
improve Primitives3D #3525
improve Primitives3D #3525
Conversation
fixed docstring in create_cone
fixed docstring in create_cone
Codecov Report
@@ Coverage Diff @@
## main #3525 +/- ##
=======================================
Coverage 81.04% 81.05%
=======================================
Files 179 179
Lines 61870 61908 +38
=======================================
+ Hits 50143 50177 +34
- Misses 11727 11731 +4 |
@tizianrot @gmalinve Please could you review the suggested modifications and push them? First update the branch please |
position into `` `` in the comments
@tizianrot there are still some things to fix in the already commited changes. |
added assert commands to check input parameters for 3D primitives
@tizianrot Please could you implement the suggestions and push again? Thanks |
"The ``position`` argument must be a valid three-element list."
for more information, see https://pre-commit.ci
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.
Thanks so much for rewording these message strings so that they follow our style guidelines! Great work!
Hi @PipKat, thanks for reviewing! I just pushed the latest commit in order to apply your advice to all messages in that class. Thanks! Giulia |
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.
@gmalinve I leave marking a PR as approved to someone with technical expertise. I only review the docstrings and prose--which looks fine. You'll need a technical team member to approve the PR.
@PipKat yes, I meant if you could review the doc since I changed other docstrings. |
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.
Great work
fixed docstrings for create_cylinder
fixed input list for create_box and create_polyhedron