iterative / dvclive

📈 Log and track ML metrics, parameters, models with Git and/or DVC
https://dvc.org/doc/dvclive
Apache License 2.0
161 stars 35 forks source link

Add docstrings to public functions #767

Closed AlexandreKempf closed 7 months ago

AlexandreKempf commented 7 months ago

This PR fix this issue: https://github.com/iterative/dvclive/issues/755

I was inspired by the dvc.org documentation for most arguments and descriptions. Let me know if you were hoping for something more technical.

I'm still struggling to find a nice description for cache, post_to_studio and save_dvc_exp. If you could enlighten me so I can finish this MR, that would be great.

I notice some missing type hints in the methods. I would love to correct those, but I'll do it on another PR.

dberenbaum commented 7 months ago

Thanks @AlexandreKempf! The one question I have is how much to make the docstring match the actual docs. It looks like you copied a lot from the existing docs but also added or changed the language in some parts.

We aren't likely to auto-generate the docs from the docstrings anytime soon (the docs include some additional info that wouldn't fit well into docstrings, and the docs are integrated with the rest of the products and we don't want to take on a project to separately generate them with a different engine). However, if we don't have some guideline on how to keep them in sync, I worry we will end up having to review two sets of docs for every change we make.

AlexandreKempf commented 7 months ago

Yeap I 100% agree @dberenbaum! There will always be a problem with syncing documentation on change if we don't autogenerate one or the other.

Maybe we need to discuss who the readers for both docs (website and docstrings) are :)

My feeling (and the way I wrote this initial PR) is: The website doc feels like it was designed for developers (technical terms, lots of code blocks), especially the API reference part we are discussing. My personal feeling is that the docstrings also target technical people. There are only two advantages over the web doc that I see:

What would you suggest the readers are? Do you see additional use for docstrings? How would you change the current PR according to these points? Do you think there is a need for docstrings or correct and extensive typing is enough?

AlexandreKempf commented 7 months ago

@dberenbaum I took your comment into account. Docstrings are now literally the same as our official doc!

shcheklein commented 7 months ago

I think one of the next steps here is to have some good quick start snippets as well or links to them in docs string ... so that people can copy / paste basic stuff. Good work, thanks @AlexandreKempf !

AlexandreKempf commented 7 months ago

@shcheklein 100% agree. Let's do that in another PR :) I'm creating an issue.

dberenbaum commented 7 months ago

@AlexandreKempf Feel free to merge approved PRs. Do you have access to merge?