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

Add docstrings to ndarray methods #205

Merged
merged 8 commits into from
Mar 1, 2022

Conversation

bryevdv
Copy link
Contributor

@bryevdv bryevdv commented Feb 28, 2022

This PR adds docstrings to ndarray methods to match the documentation structure of the official Numpy docs:

https://numpy.org/doc/stable/reference/arrays.ndarray.html

A few notes:

  • cholesky and convolve do not have direct counterparts, I wanted to solicit guidance on content before authoring any docs for these
  • Did not add docstrings for funcs that are NotImplementedError e.g. getfield and setfield

@bryevdv bryevdv added the documentation Improvements or additions to documentation label Feb 28, 2022
@magnatelee
Copy link
Contributor

cholesky and convolve do not have direct counterparts, I wanted to solicit guidance on content before authoring any docs for these

I don't think we want to expose them to the API. I feel we should rename to _cholesky and _convolve, respectively, to mean that they are internal.

Did not add docstrings for funcs that are NotImplementedError e.g. getfield and setfield

I think you made a right call there. I actually want to remove any method that does nothing more than raising that exception.

@bryevdv
Copy link
Contributor Author

bryevdv commented Mar 1, 2022

Merging, since the changes here should not intersect with any other work.

@bryevdv bryevdv merged commit dba9a1c into nv-legate:branch-22.03 Mar 1, 2022
@bryevdv bryevdv deleted the bryanv/ndarray_docstrings branch March 1, 2022 22:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants