You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "raulcd (via GitHub)" <gi...@apache.org> on 2025/04/07 13:18:23 UTC
[PR] GH-46019: [Python] Raise TypeError on feather read_table if columns is not a Sequence [arrow]
raulcd opened a new pull request, #46038:
URL: https://github.com/apache/arrow/pull/46038
### Rationale for this change
Currently if columns is a generator it fails silently and it provides a non valid result.
### What changes are included in this PR?
Check whether input is or is not a Sequence and raise the appropriate `TypeError` exception.
### Are these changes tested?
Yes
### Are there any user-facing changes?
Yes, minor change where we might raise a `TypeError` when we were not doing it before.
--
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
Re: [PR] GH-46019: [Python] Raise TypeError on feather read_table if columns is not a Sequence [arrow]
Posted by "raulcd (via GitHub)" <gi...@apache.org>.
raulcd commented on PR #46038:
URL: https://github.com/apache/arrow/pull/46038#issuecomment-2783315591
@AlenkaF I've opted to not raise materialize the generator to be consistent with other APIs were we do not support them. I think is better to be consistent here.
--
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
Re: [PR] GH-46019: [Python] Raise TypeError on feather read_table if columns is not a Sequence [arrow]
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #46038:
URL: https://github.com/apache/arrow/pull/46038#issuecomment-2790473118
After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit 577fa893681d78b0354526fe2a51db43dd3cd0bd.
There were no benchmark performance regressions. 🎉
The [full Conbench report](https://github.com/apache/arrow/runs/40270540781) has more details. It also includes information about 3 possible false positives for unstable benchmarks that are known to sometimes produce them.
--
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
Re: [PR] GH-46019: [Python] Raise TypeError on feather read_table if columns is not a Sequence [arrow]
Posted by "AlenkaF (via GitHub)" <gi...@apache.org>.
AlenkaF commented on code in PR #46038:
URL: https://github.com/apache/arrow/pull/46038#discussion_r2031276209
##########
python/pyarrow/tests/test_feather.py:
##########
@@ -805,6 +805,16 @@ def test_read_column_duplicated_in_file(tempdir):
read_table(path, columns=['a', 'b'])
+def test_read_column_wrong_column_type(tempdir, version):
+ # duplicated columns in the column selection
+ table = pa.table([[1, 2, 3], [4, 5, 6], [7, 8, 9]], names=['a', 'b', 'c'])
+ path = str(tempdir / "data.feather")
+ write_feather(table, path, version=version)
+ columns_gen = (x for x in ['a', 'b', 'c'])
+ with pytest.raises(TypeError):
Review Comment:
Could we match the message also?
```suggestion
with pytest.raises(TypeError, match="Columns must be a sequence"):
```
##########
python/pyarrow/tests/test_feather.py:
##########
@@ -805,6 +805,16 @@ def test_read_column_duplicated_in_file(tempdir):
read_table(path, columns=['a', 'b'])
+def test_read_column_wrong_column_type(tempdir, version):
+ # duplicated columns in the column selection
Review Comment:
The comment is a leftover and can be removed. As for the test name, we might change that to reflect the test? For example `test_read_column_with_generator`.
--
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
Re: [PR] GH-46019: [Python] Raise TypeError on feather read_table if columns is not a Sequence [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #46038:
URL: https://github.com/apache/arrow/pull/46038#issuecomment-2783313541
:warning: GitHub issue #46019 **has been automatically assigned in GitHub** to PR creator.
--
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
Re: [PR] GH-46019: [Python] Raise TypeError on feather read_table if columns is not a Sequence [arrow]
Posted by "raulcd (via GitHub)" <gi...@apache.org>.
raulcd merged PR #46038:
URL: https://github.com/apache/arrow/pull/46038
--
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
Re: [PR] GH-46019: [Python] Raise TypeError on feather read_table if columns is not a Sequence [arrow]
Posted by "raulcd (via GitHub)" <gi...@apache.org>.
raulcd commented on code in PR #46038:
URL: https://github.com/apache/arrow/pull/46038#discussion_r2031391850
##########
python/pyarrow/tests/test_feather.py:
##########
@@ -805,6 +805,16 @@ def test_read_column_duplicated_in_file(tempdir):
read_table(path, columns=['a', 'b'])
+def test_read_column_wrong_column_type(tempdir, version):
+ # duplicated columns in the column selection
Review Comment:
yep, sorry!
--
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