jorisvandenbossche commented on PR #13126:
URL: https://github.com/apache/arrow/pull/13126#issuecomment-1177693668
@krcrouse thanks a lot for your work on this!
I didn't yet look in detail, but some general comments:
- I think we will probably want to commit the generated code to the repo
(and in any case, doing that for now might also make it easier to review
(although you already linked the file above as well)).
- Can you explain a bit more the need for `pyarrow.docutils`? (we should
also make it a private module, or put in eg python/scripts or so?)
As far as I understand, by using the full ability of docutils, we can
override _any_ section of the docstring (eg also overriding the `Parameters`
section), instead of only _appending_ content to the docstring? (eg appending
the example of notes section).
Do we think we will really need that full ability? Or it might in almost
all cases be sufficient to just append content? (I think for all cases you
currently have additions in this PR, appending would also be fine?) If
appending is sufficient, that might keep the generation code simpler?
- > pyarrow.compute now imports from pyarrow.generated.compute
Small nit: this doesn't need to be a public module, so maybe something
like `_compute_generated.py` instead?
- > Using raw reSt so that code block examples can be tested using doctest
We now started to tests docstring examples in general in the meantime (see
https://github.com/apache/arrow/pull/13199,
https://github.com/apache/arrow/pull/13216,
https://github.com/apache/arrow/pull/13325), so we can probably test those
docstrings that way as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
