ARROW-13236: [Python] Include options class name in repr #10641
Conversation
python/pyarrow/tests/test_compute.py
Outdated
There was a problem hiding this comment.
Can you test the actual repr for one or two classes somewhere?
There was a problem hiding this comment.
I added some assertions at the bottom of the test.
python/pyarrow/tests/test_compute.py
Outdated
There was a problem hiding this comment.
Small nit, for Python it might be nice to use (..) instead of {..} (do the curly braces make more sense for C++? Also there instantiating the class uses round brackets?)
There was a problem hiding this comment.
Would <> be more typical for Python? We can slice in repr here and replace with our own braces.
For C++, you can generally use either parens or curly braces. But the only reason why curly braces are used here is because that's what Expression::ToString did before the refactor, and I assume there it's so things would print like cast(..., {...}) instead of cast(..., (...)), i.e. to not nest parentheses too much.
There was a problem hiding this comment.
() would be more typical, but here the representation isn't roundtrippable anyway.
There was a problem hiding this comment.
I'd prefer Seems like it has been updated already.(..) as well.
4 participants
No description provided.