Conversation
|
This PR now has the remaining items from issue #96 documented and is ready for review. I've updated description to automatically close out the issue upon submission. |
keenan-simpson
left a comment
There was a problem hiding this comment.
I'm a fan. I added some super minor nits that you can choose to ignore.
There was a problem hiding this comment.
LGTM overall, thanks a ton for the careful documentation effort Vlad! I took a few quick glances.
One general comment: For classes, let's not document __init__ (and certainly not __new__, which is an implementation detail). We merge everything from the constructor to the class docstring, otherwise Sphinx might be confused:
Either form is acceptable, but the two should not be mixed. Choose one
convention to document the__init__method and be consistent with it.
https://www.sphinx-doc.org/en/master/usage/extensions/example_numpy.html
Below is one example how it would look like (note that we need to manually copy the constructor signature on the first line):
https://github.com/NVIDIA/nvmath-python/blob/7c485842d0f3300e03ec780056936503913910fe/nvmath/fft/fft.py#L537-L541
Use NumPy Style Python Docstrings to maintain consistency with cuda-bindings.
Classes: - Program - ObjectCode - Kernel - LaunchConfig Functions: - launch
Done |
leofang
left a comment
There was a problem hiding this comment.
Thanks, Vlad, LGTM!
I pushed commit 7e25688 to bring the docs up-to-date. @vzhurba01 could you try to build locally and see if you encounter any issues? If not, feel free to merge!
Use NumPy Style Python Docstrings to maintain consistency with cuda-bindings.
Edit:
Close #96
Close #207