Skip to content
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

DOCS-2313: Clean up gantry, camera, board, encoder and arm pages for docs parsing #631

Merged

Conversation

sguequierre
Copy link
Contributor

@sguequierre sguequierre commented Jun 4, 2024

Questions:

  • Is there any way to manually add a link for a type in the docstrings?

@sguequierre sguequierre requested a review from a team as a code owner June 4, 2024 21:15
Copy link
Contributor

github-actions bot commented Jun 4, 2024

Warning your change may break code samples. If your change modifies any of the following functions please contact @viamrobotics/fleet-management. Thanks!

component function
base is_moving
board gpio_pin_by_name
camera get_image
encoder get_position
motor is_moving
sensor get_readings
servo get_position
arm get_end_position
gantry get_lengths
gripper is_moving
movement_sensor get_linear_acceleration
input_controller get_controls
audio get_properties
pose_tracker get_poses
power_sensor get_power
motion get_pose
vision get_properties

@andf-viam
Copy link
Contributor

andf-viam commented Jun 5, 2024

Great start! Thanks @sguequierre !

  • Yes, you should omit timeout and extra. We scrape these from the initial usage declaration of the method (first line) and not from the Properties section. I believe the SDK-team intent is to omit these SDK-wide. Only motion included them explicitly, and should likely remove them also.
  • Yes, for docs.viam.com. We can manually add a link using the python_datatype_links array in update_sdk_methods.py. For the upstream SDK docs site though, I'd ask the team here: "any way to manually provide links here outside of the automated linking that occurs during make documentation?" Sierra, you made an excellent observation that some existing generated links by this step appear incorrect: "the resource type links are a bit tricky, as far as I can tell this is done automatically by some scraper/parser and it is not always correct (See base Properties linking to audioinput).

Copy link
Member

@stuqdog stuqdog left a comment

Choose a reason for hiding this comment

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

Nice, thanks for doing this! A couple small things, but otherwise this looks great.

Comment on lines 48 to 49
extra (Optional[Dict[str, Any]]): Extra options to pass to the underlying RPC call.
timeout (Optional[float]): An option to set how long to wait (in seconds) before calling a time-out and closing the underlying RPC call.
Copy link
Member

Choose a reason for hiding this comment

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

The idiom in python is to not provide docstring comments for extra and timeout, so we should remove these (and the same comments in below methods). It looks like the same comment is used here as in motion; sorry for the confusion! The motion doc comments should probably get removed at some point.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, thank you!

@@ -72,7 +78,11 @@ async def move_to_position(
await my_arm.move_to_position(pose=examplePose)

Args:
pose (Pose): The destination Pose for the arm.
pose (Pose): The destination ``Pose`` for the arm. The ``Pose`` is composed of values for location and orientation with respect to the origin.
Copy link
Member

Choose a reason for hiding this comment

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

The linter is complaining about this line (and some others) that they're too long; can we cut this down or break it into multiple lines?

@sguequierre
Copy link
Contributor Author

  • any way to manually provide links here outside of the automated linking that occurs during make documentation?

@stuqdog do you know about this? No worries if not

@sguequierre sguequierre requested a review from stuqdog June 7, 2024 17:24
@sguequierre sguequierre changed the title DOCS-2313: Clean up gantry and arm pages for docs parsing DOCS-2313: Clean up gantry, camera, and arm pages for docs parsing Jun 7, 2024
@sguequierre sguequierre changed the title DOCS-2313: Clean up gantry, camera, and arm pages for docs parsing DOCS-2313: Clean up gantry, camera, board and arm pages for docs parsing Jun 7, 2024
@sguequierre sguequierre changed the title DOCS-2313: Clean up gantry, camera, board and arm pages for docs parsing DOCS-2313: Clean up gantry, camera, board, encoder and arm pages for docs parsing Jun 7, 2024
@stuqdog
Copy link
Member

stuqdog commented Jun 10, 2024

  • any way to manually provide links here outside of the automated linking that occurs during make documentation?

@stuqdog do you know about this? No worries if not

Ah good question! I'm not sure, but @njooma might know?

Copy link
Contributor

@andf-viam andf-viam left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@sguequierre sguequierre merged commit 921a883 into viamrobotics:main Jun 11, 2024
12 checks passed
@sguequierre sguequierre deleted the DOCS-2313/py-sdk-upstream-fixes branch June 11, 2024 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants