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-1322: Suggest parameters, returns, descriptions for missing SDK methods #2087

Conversation

sguequierre
Copy link
Collaborator

@sguequierre sguequierre commented Oct 24, 2023

Pulling more info automated

@viambot viambot added the safe to build This pull request is marked safe to build from a trusted zone label Oct 24, 2023
@sguequierre sguequierre changed the title DOCS-1322: Suggest parameters for missing SDK methods DOCS-1322: Suggest parameters and returns for missing SDK methods Oct 24, 2023
@sguequierre sguequierre marked this pull request as ready for review October 24, 2023 19:06
Copy link
Collaborator

@npentrel npentrel left a comment

Choose a reason for hiding this comment

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

you currently look for all objects that have the type "sig sig-object py". I think you could instead look for objects that have the class "py method". Then you know that the first child which also has the class "sig sig-object py" is the object where you can get the id from. But, more importantly, the second child of that object has the description text. And then you can also get the parameters with their descriptions from that I think. Can you give that a go?

@sguequierre sguequierre requested a review from npentrel October 25, 2023 16:08
@sguequierre sguequierre changed the title DOCS-1322: Suggest parameters and returns for missing SDK methods DOCS-1322: Suggest parameters, returns, descriptions for missing SDK methods Oct 25, 2023
@npentrel
Copy link
Collaborator

Nice, now do you think you could change the formatting of the output to resemble what the docs need so that it's a simple copy and paste task?

@sguequierre
Copy link
Collaborator Author

Nice, now do you think you could change the formatting of the output to resemble what the docs need so that it's a simple copy and paste task?

@npentrel Tried to format it the best I can think of, LMK what you think!

@npentrel
Copy link
Collaborator

So this is an example of what you're getting as the output

Method: viam.services.slam.client.SLAMClient.get_latest_map_info 
 Method Parameters, Returned, Description: 
Get the timestamp of the last update to the point cloud SLAM map. 
  -The timestamp of the last update. 
  -datetime

I think pulling the pieces apart and adding the markup we need for the docs should work to create something like the following output. The close we can get to something we can just copy and paste and have it work, the better

Get the timestamp of the last update to the point cloud SLAM map.

**Parameters:**
- `timeout` (Optional[float]):

**Returns:**
- `datetime` (datetime.datetime): The timestamp of the last update.

For more information, see the [Python SDK Docs](URL - which I think you can also get).

@sguequierre
Copy link
Collaborator Author

sguequierre commented Oct 26, 2023

Thank you, that makes sense! @npentrel I tried to update it to match your sample but this is the closest I could get it. They use relative links on the page and I couldn't figure out how to pull the method url precisely, tried a couple selectors, just looks like what we already have for ID so for URL I am using page url. That ok for now?

@sguequierre sguequierre requested a review from npentrel October 27, 2023 14:18
@viambot
Copy link
Member

viambot commented Oct 27, 2023

You can view a rendered version of the docs from this PR at https://docs-test.viam.dev/2087/public

@npentrel npentrel merged commit a37a892 into viamrobotics:main Oct 27, 2023
9 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
safe to build This pull request is marked safe to build from a trusted zone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants