You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/08/04 07:01:33 UTC
[GitHub] [arrow] milesgranger opened a new pull request, #13793: ARROW-16356: [Python] Expose RandomAccessFile::GetStream
milesgranger opened a new pull request, #13793:
URL: https://github.com/apache/arrow/pull/13793
This will fix [ARROW-16356](https://issues.apache.org/jira/browse/ARROW-16356), taking over from PR #13144
Adds `pyarrow.NativeFile.get_stream`, ie:
```python
with pa.PythonFile(io.BytesIO(b'data'), mode='r') as f:
stream = f.get_stream(file_offset=1, nbytes=3)
assert stream.read() == b'ata'
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] milesgranger commented on a diff in pull request #13793: ARROW-16356: [Python] Expose RandomAccessFile::GetStream
Posted by GitBox <gi...@apache.org>.
milesgranger commented on code in PR #13793:
URL: https://github.com/apache/arrow/pull/13793#discussion_r937457037
##########
python/pyarrow/tests/test_io.py:
##########
@@ -126,6 +126,44 @@ def test_python_file_read():
pa.PythonFile(StringIO(), mode='r')
+@pytest.mark.parametrize("nbytes", (0, 1, 5, 100))
+@pytest.mark.parametrize("file_offset", (0, 5, 100))
+def test_python_file_get_stream(nbytes, file_offset):
+
+ data = b'data1data2data3data4data5'
+
+ f = pa.PythonFile(BytesIO(data), mode='r')
+ stream = f.get_stream(file_offset=file_offset, nbytes=nbytes)
+
+ # Subsequent calls to 'read' should match behavior if same
+ # data passed to BytesIO where get_stream should handle if
+ # nbytes/file_offset results in no bytes b/c out of bounds.
+ start = min(file_offset, len(data))
+ end = min(file_offset + nbytes, len(data))
+ buf = BytesIO(data[start:end])
+
+ # read some chunks
+ assert stream.read(nbytes=4) == buf.read(4)
+ assert stream.read(nbytes=6) == buf.read(6)
+
+ # Read to end of each stream
+ assert stream.read() == buf.read()
+
+ # Try reading passed the stream
+ n = len(data) * 2
+ assert stream.read(n) == buf.read(n)
+
+ # NativeFile[CInputStream] is not seekable
+ with pytest.raises(OSError) as e:
+ stream.seek(0)
+
+ # some error about not being seekable
+ assert e.match("seekable")
+
+ stream.close()
+ assert stream.closed
+
+
Review Comment:
Seems like the python `get_stream` should raise ValueErrors if either of these are negative. Or at least if `file_offset` is negative right away instead of waiting until `read()` What do you think?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #13793: ARROW-16356: [Python] Expose RandomAccessFile::GetStream
Posted by GitBox <gi...@apache.org>.
pitrou commented on PR #13793:
URL: https://github.com/apache/arrow/pull/13793#issuecomment-1205384386
Hmm, looks like you need to reformat the Python and C++ code as well (see the failed Lint job).
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a diff in pull request #13793: ARROW-16356: [Python] Expose RandomAccessFile::GetStream
Posted by GitBox <gi...@apache.org>.
pitrou commented on code in PR #13793:
URL: https://github.com/apache/arrow/pull/13793#discussion_r938635339
##########
cpp/src/arrow/io/interfaces.h:
##########
@@ -262,8 +262,8 @@ class ARROW_EXPORT RandomAccessFile : public InputStream, public Seekable {
/// \param[in] file_offset the starting position in the file
/// \param[in] nbytes the extent of bytes to read. The file should have
/// sufficient bytes available
- static std::shared_ptr<InputStream> GetStream(std::shared_ptr<RandomAccessFile> file,
- int64_t file_offset, int64_t nbytes);
+ static Result<std::shared_ptr<InputStream>> GetStream(
+ std::shared_ptr<RandomAccessFile> file, int64_t file_offset, int64_t nbytes);
Review Comment:
It's not possible: you cannot have two overloads with the same exact arguments.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jorisvandenbossche commented on a diff in pull request #13793: ARROW-16356: [Python] Expose RandomAccessFile::GetStream
Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on code in PR #13793:
URL: https://github.com/apache/arrow/pull/13793#discussion_r938634613
##########
cpp/src/arrow/io/interfaces.h:
##########
@@ -262,8 +262,8 @@ class ARROW_EXPORT RandomAccessFile : public InputStream, public Seekable {
/// \param[in] file_offset the starting position in the file
/// \param[in] nbytes the extent of bytes to read. The file should have
/// sufficient bytes available
- static std::shared_ptr<InputStream> GetStream(std::shared_ptr<RandomAccessFile> file,
- int64_t file_offset, int64_t nbytes);
+ static Result<std::shared_ptr<InputStream>> GetStream(
+ std::shared_ptr<RandomAccessFile> file, int64_t file_offset, int64_t nbytes);
Review Comment:
@pitrou should we keep the original non-Result signature as well for backwards compatibility?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] milesgranger commented on a diff in pull request #13793: ARROW-16356: [Python] Expose RandomAccessFile::GetStream
Posted by GitBox <gi...@apache.org>.
milesgranger commented on code in PR #13793:
URL: https://github.com/apache/arrow/pull/13793#discussion_r937548346
##########
python/pyarrow/tests/test_io.py:
##########
@@ -126,6 +126,44 @@ def test_python_file_read():
pa.PythonFile(StringIO(), mode='r')
+@pytest.mark.parametrize("nbytes", (0, 1, 5, 100))
+@pytest.mark.parametrize("file_offset", (0, 5, 100))
+def test_python_file_get_stream(nbytes, file_offset):
+
+ data = b'data1data2data3data4data5'
+
+ f = pa.PythonFile(BytesIO(data), mode='r')
+ stream = f.get_stream(file_offset=file_offset, nbytes=nbytes)
+
+ # Subsequent calls to 'read' should match behavior if same
+ # data passed to BytesIO where get_stream should handle if
+ # nbytes/file_offset results in no bytes b/c out of bounds.
+ start = min(file_offset, len(data))
+ end = min(file_offset + nbytes, len(data))
+ buf = BytesIO(data[start:end])
+
+ # read some chunks
+ assert stream.read(nbytes=4) == buf.read(4)
+ assert stream.read(nbytes=6) == buf.read(6)
+
+ # Read to end of each stream
+ assert stream.read() == buf.read()
+
+ # Try reading passed the stream
+ n = len(data) * 2
+ assert stream.read(n) == buf.read(n)
+
+ # NativeFile[CInputStream] is not seekable
+ with pytest.raises(OSError) as e:
+ stream.seek(0)
+
+ # some error about not being seekable
+ assert e.match("seekable")
+
+ stream.close()
+ assert stream.closed
+
+
Review Comment:
Gave it a go in https://github.com/apache/arrow/pull/13793/commits/eae061be98968e109e54008c82733e0174962989 Suppose we can resolve this thread and inevitably open another where the cpp changes are then.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a diff in pull request #13793: ARROW-16356: [Python] Expose RandomAccessFile::GetStream
Posted by GitBox <gi...@apache.org>.
pitrou commented on code in PR #13793:
URL: https://github.com/apache/arrow/pull/13793#discussion_r937468765
##########
python/pyarrow/tests/test_io.py:
##########
@@ -126,6 +126,44 @@ def test_python_file_read():
pa.PythonFile(StringIO(), mode='r')
+@pytest.mark.parametrize("nbytes", (0, 1, 5, 100))
+@pytest.mark.parametrize("file_offset", (0, 5, 100))
+def test_python_file_get_stream(nbytes, file_offset):
+
+ data = b'data1data2data3data4data5'
+
+ f = pa.PythonFile(BytesIO(data), mode='r')
+ stream = f.get_stream(file_offset=file_offset, nbytes=nbytes)
+
+ # Subsequent calls to 'read' should match behavior if same
+ # data passed to BytesIO where get_stream should handle if
+ # nbytes/file_offset results in no bytes b/c out of bounds.
+ start = min(file_offset, len(data))
+ end = min(file_offset + nbytes, len(data))
+ buf = BytesIO(data[start:end])
+
+ # read some chunks
+ assert stream.read(nbytes=4) == buf.read(4)
+ assert stream.read(nbytes=6) == buf.read(6)
+
+ # Read to end of each stream
+ assert stream.read() == buf.read()
+
+ # Try reading passed the stream
+ n = len(data) * 2
+ assert stream.read(n) == buf.read(n)
+
+ # NativeFile[CInputStream] is not seekable
+ with pytest.raises(OSError) as e:
+ stream.seek(0)
+
+ # some error about not being seekable
+ assert e.match("seekable")
+
+ stream.close()
+ assert stream.closed
+
+
Review Comment:
Raising as soon as `get_stream` is called would be preferable indeed, but it should probably be done on the C++ side
(which is a good introduction for you 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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ursabot commented on pull request #13793: ARROW-16356: [Python] Expose RandomAccessFile::GetStream
Posted by GitBox <gi...@apache.org>.
ursabot commented on PR #13793:
URL: https://github.com/apache/arrow/pull/13793#issuecomment-1208098568
Benchmark runs are scheduled for baseline = a9a8626e8342e1145ec1b60ffb50154693b9ef92 and contender = 5719576c611929dd790f7f8a1ae3169a8f96f7f1. 5719576c611929dd790f7f8a1ae3169a8f96f7f1 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/fe11871da44f407ab34641b03989873d...67ebda53d7824fcf99bf46552bdecb11/)
[Failed :arrow_down:0.34% :arrow_up:0.0%] [test-mac-arm](https://conbench.ursa.dev/compare/runs/204cefc84bf64ddbafffcc6233d02596...a1212bfcb0654036bdfe3eacd920f718/)
[Finished :arrow_down:0.27% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/ac55b04a9a0e400eb475f0577a46b884...f565a38be2864a75b596db533edec2a8/)
[Finished :arrow_down:0.85% :arrow_up:0.0%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/2714054f5a8a4e22a75e7096b52f9002...bd907a4d276e4773bbeab3e765de8ff8/)
Buildkite builds:
[Finished] [`5719576c` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/1270)
[Failed] [`5719576c` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/1282)
[Finished] [`5719576c` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/1265)
[Finished] [`5719576c` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/1284)
[Finished] [`a9a8626e` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/1269)
[Failed] [`a9a8626e` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/1281)
[Finished] [`a9a8626e` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/1264)
[Finished] [`a9a8626e` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/1283)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #13793: ARROW-16356: [Python] Expose RandomAccessFile::GetStream
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #13793:
URL: https://github.com/apache/arrow/pull/13793#issuecomment-1204847487
https://issues.apache.org/jira/browse/ARROW-16356
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] milesgranger commented on a diff in pull request #13793: ARROW-16356: [Python] Expose RandomAccessFile::GetStream
Posted by GitBox <gi...@apache.org>.
milesgranger commented on code in PR #13793:
URL: https://github.com/apache/arrow/pull/13793#discussion_r937434802
##########
python/pyarrow/tests/test_io.py:
##########
@@ -126,6 +126,44 @@ def test_python_file_read():
pa.PythonFile(StringIO(), mode='r')
+@pytest.mark.parametrize("nbytes", (0, 1, 5, 100))
+@pytest.mark.parametrize("file_offset", (0, 5, 100))
+def test_python_file_get_stream(nbytes, file_offset):
+
+ data = b'data1data2data3data4data5'
+
+ f = pa.PythonFile(BytesIO(data), mode='r')
+ stream = f.get_stream(file_offset=file_offset, nbytes=nbytes)
+
+ # Subsequent calls to 'read' should match behavior if same
+ # data passed to BytesIO where get_stream should handle if
+ # nbytes/file_offset results in no bytes b/c out of bounds.
+ start = min(file_offset, len(data))
+ end = min(file_offset + nbytes, len(data))
+ buf = BytesIO(data[start:end])
+
+ # read some chunks
+ assert stream.read(nbytes=4) == buf.read(4)
+ assert stream.read(nbytes=6) == buf.read(6)
+
+ # Read to end of each stream
+ assert stream.read() == buf.read()
+
+ # Try reading passed the stream
Review Comment:
...and I'm an aspiring one [it seems](https://www.dailywritingtips.com/passed-vs-past/) :sweat_smile:
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou merged pull request #13793: ARROW-16356: [Python] Expose RandomAccessFile::GetStream
Posted by GitBox <gi...@apache.org>.
pitrou merged PR #13793:
URL: https://github.com/apache/arrow/pull/13793
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a diff in pull request #13793: ARROW-16356: [Python] Expose RandomAccessFile::GetStream
Posted by GitBox <gi...@apache.org>.
pitrou commented on code in PR #13793:
URL: https://github.com/apache/arrow/pull/13793#discussion_r937424893
##########
python/pyarrow/tests/test_io.py:
##########
@@ -126,6 +126,44 @@ def test_python_file_read():
pa.PythonFile(StringIO(), mode='r')
+@pytest.mark.parametrize("nbytes", (0, 1, 5, 100))
+@pytest.mark.parametrize("file_offset", (0, 5, 100))
+def test_python_file_get_stream(nbytes, file_offset):
+
+ data = b'data1data2data3data4data5'
+
+ f = pa.PythonFile(BytesIO(data), mode='r')
+ stream = f.get_stream(file_offset=file_offset, nbytes=nbytes)
+
+ # Subsequent calls to 'read' should match behavior if same
+ # data passed to BytesIO where get_stream should handle if
+ # nbytes/file_offset results in no bytes b/c out of bounds.
+ start = min(file_offset, len(data))
+ end = min(file_offset + nbytes, len(data))
+ buf = BytesIO(data[start:end])
+
+ # read some chunks
+ assert stream.read(nbytes=4) == buf.read(4)
+ assert stream.read(nbytes=6) == buf.read(6)
+
+ # Read to end of each stream
+ assert stream.read() == buf.read()
+
+ # Try reading passed the stream
Review Comment:
I think there is a misspelling? (not a native English speaker though...)
```suggestion
# Try reading past the stream
```
##########
python/pyarrow/tests/test_io.py:
##########
@@ -126,6 +126,44 @@ def test_python_file_read():
pa.PythonFile(StringIO(), mode='r')
+@pytest.mark.parametrize("nbytes", (0, 1, 5, 100))
+@pytest.mark.parametrize("file_offset", (0, 5, 100))
+def test_python_file_get_stream(nbytes, file_offset):
+
+ data = b'data1data2data3data4data5'
+
+ f = pa.PythonFile(BytesIO(data), mode='r')
+ stream = f.get_stream(file_offset=file_offset, nbytes=nbytes)
+
+ # Subsequent calls to 'read' should match behavior if same
+ # data passed to BytesIO where get_stream should handle if
+ # nbytes/file_offset results in no bytes b/c out of bounds.
+ start = min(file_offset, len(data))
+ end = min(file_offset + nbytes, len(data))
+ buf = BytesIO(data[start:end])
+
+ # read some chunks
+ assert stream.read(nbytes=4) == buf.read(4)
+ assert stream.read(nbytes=6) == buf.read(6)
+
+ # Read to end of each stream
+ assert stream.read() == buf.read()
+
+ # Try reading passed the stream
+ n = len(data) * 2
+ assert stream.read(n) == buf.read(n)
+
+ # NativeFile[CInputStream] is not seekable
+ with pytest.raises(OSError) as e:
+ stream.seek(0)
+
+ # some error about not being seekable
+ assert e.match("seekable")
+
+ stream.close()
+ assert stream.closed
+
+
Review Comment:
A couple more questions: what happens if either the offset or the length is negative?
##########
python/pyarrow/tests/test_io.py:
##########
@@ -126,6 +126,44 @@ def test_python_file_read():
pa.PythonFile(StringIO(), mode='r')
+@pytest.mark.parametrize("nbytes", (0, 1, 5, 100))
+@pytest.mark.parametrize("file_offset", (0, 5, 100))
+def test_python_file_get_stream(nbytes, file_offset):
+
+ data = b'data1data2data3data4data5'
+
+ f = pa.PythonFile(BytesIO(data), mode='r')
+ stream = f.get_stream(file_offset=file_offset, nbytes=nbytes)
+
+ # Subsequent calls to 'read' should match behavior if same
+ # data passed to BytesIO where get_stream should handle if
+ # nbytes/file_offset results in no bytes b/c out of bounds.
+ start = min(file_offset, len(data))
+ end = min(file_offset + nbytes, len(data))
+ buf = BytesIO(data[start:end])
+
+ # read some chunks
+ assert stream.read(nbytes=4) == buf.read(4)
+ assert stream.read(nbytes=6) == buf.read(6)
+
+ # Read to end of each stream
+ assert stream.read() == buf.read()
+
+ # Try reading passed the stream
+ n = len(data) * 2
+ assert stream.read(n) == buf.read(n)
+
+ # NativeFile[CInputStream] is not seekable
+ with pytest.raises(OSError) as e:
+ stream.seek(0)
+
+ # some error about not being seekable
+ assert e.match("seekable")
Review Comment:
This can be written a bit more succinctly:
```suggestion
# NativeFile[CInputStream] is not seekable
with pytest.raises(OSError, match="seekable") as e:
stream.seek(0)
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] milesgranger commented on a diff in pull request #13793: ARROW-16356: [Python] Expose RandomAccessFile::GetStream
Posted by GitBox <gi...@apache.org>.
milesgranger commented on code in PR #13793:
URL: https://github.com/apache/arrow/pull/13793#discussion_r937447368
##########
python/pyarrow/tests/test_io.py:
##########
@@ -126,6 +126,44 @@ def test_python_file_read():
pa.PythonFile(StringIO(), mode='r')
+@pytest.mark.parametrize("nbytes", (0, 1, 5, 100))
+@pytest.mark.parametrize("file_offset", (0, 5, 100))
+def test_python_file_get_stream(nbytes, file_offset):
+
+ data = b'data1data2data3data4data5'
+
+ f = pa.PythonFile(BytesIO(data), mode='r')
+ stream = f.get_stream(file_offset=file_offset, nbytes=nbytes)
+
+ # Subsequent calls to 'read' should match behavior if same
+ # data passed to BytesIO where get_stream should handle if
+ # nbytes/file_offset results in no bytes b/c out of bounds.
+ start = min(file_offset, len(data))
+ end = min(file_offset + nbytes, len(data))
+ buf = BytesIO(data[start:end])
+
+ # read some chunks
+ assert stream.read(nbytes=4) == buf.read(4)
+ assert stream.read(nbytes=6) == buf.read(6)
+
+ # Read to end of each stream
+ assert stream.read() == buf.read()
+
+ # Try reading passed the stream
+ n = len(data) * 2
+ assert stream.read(n) == buf.read(n)
+
+ # NativeFile[CInputStream] is not seekable
+ with pytest.raises(OSError) as e:
+ stream.seek(0)
+
+ # some error about not being seekable
+ assert e.match("seekable")
+
+ stream.close()
+ assert stream.closed
+
+
Review Comment:
Added in https://github.com/apache/arrow/pull/13793/commits/079ce14406204c1474ff182d70a1360ecedd9ab5
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #13793: ARROW-16356: [Python] Expose RandomAccessFile::GetStream
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #13793:
URL: https://github.com/apache/arrow/pull/13793#issuecomment-1204847510
:warning: Ticket **has not been started in JIRA**, please click 'Start Progress'.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] milesgranger commented on a diff in pull request #13793: ARROW-16356: [Python] Expose RandomAccessFile::GetStream
Posted by GitBox <gi...@apache.org>.
milesgranger commented on code in PR #13793:
URL: https://github.com/apache/arrow/pull/13793#discussion_r937433923
##########
python/pyarrow/tests/test_io.py:
##########
@@ -126,6 +126,44 @@ def test_python_file_read():
pa.PythonFile(StringIO(), mode='r')
+@pytest.mark.parametrize("nbytes", (0, 1, 5, 100))
+@pytest.mark.parametrize("file_offset", (0, 5, 100))
+def test_python_file_get_stream(nbytes, file_offset):
+
+ data = b'data1data2data3data4data5'
+
+ f = pa.PythonFile(BytesIO(data), mode='r')
+ stream = f.get_stream(file_offset=file_offset, nbytes=nbytes)
+
+ # Subsequent calls to 'read' should match behavior if same
+ # data passed to BytesIO where get_stream should handle if
+ # nbytes/file_offset results in no bytes b/c out of bounds.
+ start = min(file_offset, len(data))
+ end = min(file_offset + nbytes, len(data))
+ buf = BytesIO(data[start:end])
+
+ # read some chunks
+ assert stream.read(nbytes=4) == buf.read(4)
+ assert stream.read(nbytes=6) == buf.read(6)
+
+ # Read to end of each stream
+ assert stream.read() == buf.read()
+
+ # Try reading passed the stream
+ n = len(data) * 2
+ assert stream.read(n) == buf.read(n)
+
+ # NativeFile[CInputStream] is not seekable
+ with pytest.raises(OSError) as e:
+ stream.seek(0)
+
+ # some error about not being seekable
+ assert e.match("seekable")
+
+ stream.close()
+ assert stream.closed
+
+
Review Comment:
A `ValueError` for negative file_offset, and appears a negative `nbytes` is the same as wanting all the remaining data.
```python
>>> with pa.PythonFile(io.BytesIO(b'data'), mode='r') as f:
... stream = f.get_stream(file_offset=-1, nbytes=3)
... stream.read()
...
Traceback (most recent call last):
File "<stdin>", line 3, in <module>
File "pyarrow/io.pxi", line 378, in pyarrow.lib.NativeFile.read
chunk = self.read(bs)
File "pyarrow/io.pxi", line 395, in pyarrow.lib.NativeFile.read
bytes_read = GetResultValue(handle.get().Read(c_nbytes, buf))
File "pyarrow/error.pxi", line 144, in pyarrow.lib.pyarrow_internal_check_status
return check_status(status)
ValueError: negative seek value -1
>>>
>>> with pa.PythonFile(io.BytesIO(b'data'), mode='r') as f:
... stream = f.get_stream(file_offset=1, nbytes=-100)
... stream.read()
...
b'ata'
```
I can add to the test to check `ValueError` for negative `file_offset` and negative `nbytes` is same as all data?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] milesgranger commented on a diff in pull request #13793: ARROW-16356: [Python] Expose RandomAccessFile::GetStream
Posted by GitBox <gi...@apache.org>.
milesgranger commented on code in PR #13793:
URL: https://github.com/apache/arrow/pull/13793#discussion_r937447368
##########
python/pyarrow/tests/test_io.py:
##########
@@ -126,6 +126,44 @@ def test_python_file_read():
pa.PythonFile(StringIO(), mode='r')
+@pytest.mark.parametrize("nbytes", (0, 1, 5, 100))
+@pytest.mark.parametrize("file_offset", (0, 5, 100))
+def test_python_file_get_stream(nbytes, file_offset):
+
+ data = b'data1data2data3data4data5'
+
+ f = pa.PythonFile(BytesIO(data), mode='r')
+ stream = f.get_stream(file_offset=file_offset, nbytes=nbytes)
+
+ # Subsequent calls to 'read' should match behavior if same
+ # data passed to BytesIO where get_stream should handle if
+ # nbytes/file_offset results in no bytes b/c out of bounds.
+ start = min(file_offset, len(data))
+ end = min(file_offset + nbytes, len(data))
+ buf = BytesIO(data[start:end])
+
+ # read some chunks
+ assert stream.read(nbytes=4) == buf.read(4)
+ assert stream.read(nbytes=6) == buf.read(6)
+
+ # Read to end of each stream
+ assert stream.read() == buf.read()
+
+ # Try reading passed the stream
+ n = len(data) * 2
+ assert stream.read(n) == buf.read(n)
+
+ # NativeFile[CInputStream] is not seekable
+ with pytest.raises(OSError) as e:
+ stream.seek(0)
+
+ # some error about not being seekable
+ assert e.match("seekable")
+
+ stream.close()
+ assert stream.closed
+
+
Review Comment:
~Added in https://github.com/apache/arrow/pull/13793/commits/079ce14406204c1474ff182d70a1360ecedd9ab5~
Need to adjust, didn't add nbytes to have a negative value in the test.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #13793: ARROW-16356: [Python] Expose RandomAccessFile::GetStream
Posted by GitBox <gi...@apache.org>.
pitrou commented on PR #13793:
URL: https://github.com/apache/arrow/pull/13793#issuecomment-1205007449
Uh, it looks like AppVeyor is complaining for an unrelated issue: #13795
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #13793: ARROW-16356: [Python] Expose RandomAccessFile::GetStream
Posted by GitBox <gi...@apache.org>.
pitrou commented on PR #13793:
URL: https://github.com/apache/arrow/pull/13793#issuecomment-1205173761
So most CI looks fine, can you rebase or merge from master to get a fix for AppVeyor?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org