-
Notifications
You must be signed in to change notification settings - Fork 4k
GH-31507: [Python] Address docstrings in Streams and File Access (Stream Classes) #33698
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
Conversation
jorisvandenbossche
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few small comments!
python/pyarrow/io.pxi
Outdated
| >>> import pyarrow as pa | ||
| >>> data = b'reader data' | ||
| >>> f = pa.BufferReader(data) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should maybe also reference pa.input_stream() as the (preferred?) way to create BufferReader objects?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(and same for some of the others)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added changes regarding this comment in this commit: 10d5e89
I hope I captured all the necessary changes.
Co-authored-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
| >>> data = b'reader data' | ||
| >>> f = pa.BufferReader(data) | ||
| >>> f.size() | ||
| >>> buf = memoryview(data) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the memoryview(..) needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It needs some kind of a source (str, Path, buffer, file-like object, …):
>>> import pyarrow as pa
>>> data = b'reader data'
>>> with pa.input_stream(data) as stream:
... stream.size()
...
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "pyarrow/io.pxi", line 2403, in pyarrow.lib.input_stream
hasattr(source, 'closed')):
TypeError: pa.input_stream() called with instance of '<class 'bytes'>'Can change the source, if preferred.
|
I ran through the examples and they all worked well for me. Nice work! LGTM (Looks Good To Me). |
|
Benchmark runs are scheduled for baseline = 7db7e6a and contender = f2d632e. f2d632e is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Rationale for this change
Ensure docstrings for Streams and File Access - Stream Classes - have an Examples section.
What changes are included in this PR?
Docstrings are added to listed Stream Classes:
Are these changes tested?
Yes, locally with
pytest --doctest-cython --disable-warnings pyarrowand on the CI withPython / AMD64 Conda Python 3.9 Sphinx & Numpydocbuild.Are there any user-facing changes?
No.