sslivkoff
commented
9 months ago yea there is a lot of room for improvement on the current docstrings. I think this PR goes in the right direction but a couple things I would change
- the functions have a very large number of parameters, so need to consider whether it's even worth listing all of them. the alternative is just listing some of them and then providing either a url to the full list or a separate way to print out the full list
- the formatting of the parameters that are included should be more succinct. 1 line per parameter max. I've never been a fan of numpy/google style docstrings, it's too verbose for situations like these
- the indenting on the dataset list looks a bit off? and do you think it would be good to include all datasets here or just a subset?
- don't want to use black-style double quotes for the formatting. currently using ruff with single quote config'd, but haven't gotten around to adding that to pyproject.toml yet
Motivation
There are a lot of questions about how to implement cryo_python. It seems that most people either don't know what paras to use or they have slightly incorrect syntax. There is a lot of documentation about cryo_cli but implementation in python is slightly different and it is tripping people up.
Solution
Adding a more descriptive docstring for cryo_python.freeze
Requested Feedback
I'm looking for:
I think that I have all params listed but haven't added a type or description to all of them. I started looking at _cryo_rust.parse_kwargs() and saw that there were more params and I realized I was starting to go deeper and deeper... but I realized I need your feedback on what I'm implementing before I fall too far into this rabbit-hole.
Thanks in advance for taking a look and I look forward to reviewing feedback @sslivkoff