You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "thisisnic (via GitHub)" <gi...@apache.org> on 2025/08/21 17:00:29 UTC
[PR] GH-47389: [Python] CSV and JSON options lack a nice repr/str [arrow]
thisisnic opened a new pull request, #47397:
URL: https://github.com/apache/arrow/pull/47397
### Rationale for this change
CSV and JSON options lack a nice repr/str dunder method
### What changes are included in this PR?
Add both these methods
### Are these changes tested?
Will be once it's ready for review
### Are there any user-facing changes?
No
--
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-47389: [Python] CSV and JSON options lack a nice repr/str [arrow]
Posted by "AlenkaF (via GitHub)" <gi...@apache.org>.
AlenkaF commented on PR #47397:
URL: https://github.com/apache/arrow/pull/47397#issuecomment-3441437252
@pitrou mind having a quick look?
--
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-47389: [Python] CSV and JSON options lack a nice repr/str [arrow]
Posted by "thisisnic (via GitHub)" <gi...@apache.org>.
thisisnic commented on code in PR #47397:
URL: https://github.com/apache/arrow/pull/47397#discussion_r2915865856
##########
python/pyarrow/_csv.pyx:
##########
@@ -332,6 +332,22 @@ cdef class ReadOptions(_Weakrefable):
except TypeError:
return False
+ def _repr_base(self):
+ return (f"""
+use_threads={self.use_threads}
+block_size={self.block_size}
+skip_rows={self.skip_rows}
+skip_rows_after_names={self.skip_rows_after_names}
+column_names={self.column_names}
+autogenerate_column_names={self.autogenerate_column_names}
+encoding='{self.encoding}'""")
+
+ def __repr__(self):
+ return (f"<pyarrow.csv.ReadOptions>({self._repr_base()})")
Review Comment:
Updated this now!
--
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-47389: [Python] CSV and JSON options lack a nice repr/str [arrow]
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #47397:
URL: https://github.com/apache/arrow/pull/47397#issuecomment-4127214535
After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit 618d5fbe84ba417123b0341bc1f255a014128311.
There were no benchmark performance regressions. 🎉
The [full Conbench report](https://github.com/apache/arrow/runs/68550938244) has more details. It also includes information about 2 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-47389: [Python] CSV and JSON options lack a nice repr/str [arrow]
Posted by "thisisnic (via GitHub)" <gi...@apache.org>.
thisisnic commented on code in PR #47397:
URL: https://github.com/apache/arrow/pull/47397#discussion_r2313435394
##########
python/pyarrow/_csv.pyx:
##########
@@ -585,6 +605,24 @@ cdef class ParseOptions(_Weakrefable):
except TypeError:
return False
+ def __repr__(self):
+ return (f"<pyarrow.csv.ParseOptions("
+ f"delimiter={self.delimiter!r}, "
+ f"quote_char={self.quote_char!r}, "
+ f"double_quote={self.double_quote}, "
+ f"escape_char={self.escape_char!r}, "
+ f"newlines_in_values={self.newlines_in_values}, "
+ f"ignore_empty_lines={self.ignore_empty_lines})>")
Review Comment:
Oops, my bad, totally missed it
--
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-47389: [Python] CSV and JSON options lack a nice repr/str [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #47397:
URL: https://github.com/apache/arrow/pull/47397#discussion_r2469655509
##########
python/pyarrow/_csv.pyx:
##########
@@ -332,6 +332,22 @@ cdef class ReadOptions(_Weakrefable):
except TypeError:
return False
+ def _repr_base(self):
+ return (f"""
+use_threads={self.use_threads}
+block_size={self.block_size}
+skip_rows={self.skip_rows}
+skip_rows_after_names={self.skip_rows_after_names}
+column_names={self.column_names}
+autogenerate_column_names={self.autogenerate_column_names}
+encoding='{self.encoding}'""")
+
+ def __repr__(self):
+ return (f"<pyarrow.csv.ReadOptions>({self._repr_base()})")
Review Comment:
IIUC this gives something like:
```
<pyarrow.csv.ReadOptions>(
use_threads=True
block_size=1048576)
```
Can the inside of the parentheses be indented perhaps, and some commas be added? Such as:
```
<pyarrow.csv.ReadOptions>(
use_threads=True,
block_size=1048576)
```
--
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-47389: [Python] CSV and JSON options lack a nice repr/str [arrow]
Posted by "AlenkaF (via GitHub)" <gi...@apache.org>.
AlenkaF commented on code in PR #47397:
URL: https://github.com/apache/arrow/pull/47397#discussion_r2315291406
##########
python/pyarrow/_csv.pyx:
##########
@@ -1108,6 +1148,38 @@ cdef class ConvertOptions(_Weakrefable):
except TypeError:
return False
+ def __repr__(self):
Review Comment:
Here's one idea that might help simplify the current setup (also matching more closely with existing `__repr__` [examples](https://github.com/apache/arrow/blob/55be8c011cd9e82707382d97417b0c3a0aabf822/python/pyarrow/_parquet.pyx#L73-L83):
```python
def _repr_base(self):
return (f"""
check_utf8={self.check_utf8}
column_types={self.column_types}
null_values={self.null_values}
true_values={self.true_values}
false_values={self.false_values}
decimal_point={self.decimal_point!r}
strings_can_be_null={self.strings_can_be_null}
quoted_strings_can_be_null={self.quoted_strings_can_be_null}
include_columns={self.include_columns}
include_missing_columns={self.include_missing_columns}
auto_dict_encode={self.auto_dict_encode}
auto_dict_max_cardinality={self.auto_dict_max_cardinality}
timestamp_parsers={self.timestamp_parsers}""")
def __repr__(self):
return (f"{object.__repr__(self)}{self._repr_base()}")
def __str__(self):
return (f"ConvertOptions({self._repr_base()})")
```
<details>
<summary>Output example</summary>
```
<pyarrow._csv.ConvertOptions object at 0x1147ea040>
check_utf8=True
column_types={'a': DataType(null)}
null_values=['N', 'nn']
true_values=['T', 'tt']
false_values=['F', 'ff']
decimal_point='.'
strings_can_be_null=False
quoted_strings_can_be_null=True
include_columns=[]
include_missing_columns=False
auto_dict_encode=False
auto_dict_max_cardinality=999
timestamp_parsers=[<pyarrow._csv._ISO8601 object at 0x10cd360d0>, '%Y-%m-%d']
```
and
```python
ConvertOptions(
check_utf8=True
column_types={'a': DataType(null)}
null_values=['N', 'nn']
true_values=['T', 'tt']
false_values=['F', 'ff']
decimal_point='.'
strings_can_be_null=False
quoted_strings_can_be_null=True
include_columns=[]
include_missing_columns=False
auto_dict_encode=False
auto_dict_max_cardinality=999
timestamp_parsers=[<pyarrow._csv._ISO8601 object at 0x10cd360d0>, '%Y-%m-%d'])
```
</details>
Something similar to this idea can be done for all the classes (and for the tests).
##########
python/pyarrow/_csv.pyx:
##########
@@ -585,6 +605,26 @@ cdef class ParseOptions(_Weakrefable):
except TypeError:
return False
+ def __repr__(self):
+ return (f"<pyarrow.csv.ParseOptions("
+ f"delimiter={self.delimiter!r}, "
+ f"quote_char={self.quote_char!r}, "
+ f"double_quote={self.double_quote}, "
+ f"escape_char={self.escape_char!r}, "
+ f"newlines_in_values={self.newlines_in_values}, "
+ f"ignore_empty_lines={self.ignore_empty_lines}, "
+ f"invalid_row_handler={self.invalid_row_handler})>")
+
+ def __str__(self):
+ return (f"ParseOptions("
+ f"delimiter={self.delimiter!r}, "
+ f"quote_char={self.quote_char!r}, "
+ f"double_quote={self.double_quote}, "
+ f"escape_char={self.escape_char!r}, "
+ f"newlines_in_values={self.newlines_in_values}, "
+ f"ignore_empty_lines={self.ignore_empty_lines}, "
+ f"invalid_row_handler={self.invalid_row_handler})")
Review Comment:
Thanks for adding this! One thing that I am thinking now is if `invalid_row_handler` and also `timestamp_parsers` could be more readable if not `None`:
```python
timestamp_parsers=[<pyarrow._csv._ISO8601 object at 0x10cd360d0>, '%Y-%m-%d']
```
```python
invalid_row_handler=<pyarrow.tests.test_csv.InvalidRowHandler object at 0x11e828050>
```
Not a blocker, of course, but maybe there's a cleaner way to represent these in the output - not sure off the top of my head though!
--
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-47389: [Python] CSV and JSON options lack a nice repr/str [arrow]
Posted by "AlenkaF (via GitHub)" <gi...@apache.org>.
AlenkaF commented on PR #47397:
URL: https://github.com/apache/arrow/pull/47397#issuecomment-4124734953
Thanks @thisisnic !
--
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-47389: [Python] CSV and JSON options lack a nice repr/str [arrow]
Posted by "AlenkaF (via GitHub)" <gi...@apache.org>.
AlenkaF commented on code in PR #47397:
URL: https://github.com/apache/arrow/pull/47397#discussion_r2313107543
##########
python/pyarrow/tests/test_csv.py:
##########
@@ -378,6 +448,21 @@ def test_write_options():
opts.batch_size = 0
opts.validate()
+ expected_repr = ("<pyarrow.csv.WriteOptions("
+ f"include_header={opts.include_header}, "
+ f"batch_size={opts.batch_size}, "
+ f"delimiter={opts.delimiter!r}, "
+ f"quoting_style='{opts.quoting_style}')>")
+ assert repr(opts) == expected_repr
+
+ # Test str
Review Comment:
```suggestion
```
##########
python/pyarrow/_csv.pyx:
##########
@@ -585,6 +605,24 @@ cdef class ParseOptions(_Weakrefable):
except TypeError:
return False
+ def __repr__(self):
+ return (f"<pyarrow.csv.ParseOptions("
+ f"delimiter={self.delimiter!r}, "
+ f"quote_char={self.quote_char!r}, "
+ f"double_quote={self.double_quote}, "
+ f"escape_char={self.escape_char!r}, "
+ f"newlines_in_values={self.newlines_in_values}, "
+ f"ignore_empty_lines={self.ignore_empty_lines})>")
Review Comment:
I'm wondering if we should include `invalid_row_handler` — at least to indicate whether it's set or `None`?
--
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-47389: [Python] CSV and JSON options lack a nice repr/str [arrow]
Posted by "AlenkaF (via GitHub)" <gi...@apache.org>.
AlenkaF commented on PR #47397:
URL: https://github.com/apache/arrow/pull/47397#issuecomment-4089534683
Thanks for the changes @thisisnic!
I plan to merge this PR today/tomorrow in case there are no other comments.
--
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-47389: [Python] CSV and JSON options lack a nice repr/str [arrow]
Posted by "AlenkaF (via GitHub)" <gi...@apache.org>.
AlenkaF merged PR #47397:
URL: https://github.com/apache/arrow/pull/47397
--
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