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 2021/12/06 18:37:23 UTC
[GitHub] [arrow] marlenezw opened a new pull request #11873: Exposing the `indent_size` paramenter in cython
marlenezw opened a new pull request #11873:
URL: https://github.com/apache/arrow/pull/11873
This PR closes ARROW-14242.
This allows the expected behavior of `ident` in the `to_string` function to be able to indent the items and then for the top_level_indent parameter to indent the entire string.
--
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 change in pull request #11873: ARROW-14242: Exposing the correct `indent` paramenter in `to_string`
Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on a change in pull request #11873:
URL: https://github.com/apache/arrow/pull/11873#discussion_r765922898
##########
File path: python/pyarrow/array.pxi
##########
@@ -1005,16 +1005,19 @@ cdef class Array(_PandasConvertible):
type_format = object.__repr__(self)
return '{0}\n{1}'.format(type_format, str(self))
- def to_string(self, *, int indent=0, int window=10,
+ def to_string(self, *, int indent=2, int top_level_indent=0, int window=10,
c_bool skip_new_lines=False):
"""
Render a "pretty-printed" string representation of the Array.
Parameters
----------
indent : int
Review comment:
```suggestion
indent : int, default 2
```
##########
File path: python/pyarrow/array.pxi
##########
@@ -1005,16 +1005,19 @@ cdef class Array(_PandasConvertible):
type_format = object.__repr__(self)
return '{0}\n{1}'.format(type_format, str(self))
- def to_string(self, *, int indent=0, int window=10,
+ def to_string(self, *, int indent=2, int top_level_indent=0, int window=10,
c_bool skip_new_lines=False):
"""
Render a "pretty-printed" string representation of the Array.
Parameters
----------
indent : int
- How much to indent right the content of the array,
- by default ``0``.
+ How much to indent the internal items in the string to
+ the right, by default ``2``.
+ top_level_indent : int
Review comment:
```suggestion
top_level_indent : int, default 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] marlenezw commented on pull request #11873: ARROW-14242: Exposing the correct `indent` paramenter in `to_string`
Posted by GitBox <gi...@apache.org>.
marlenezw commented on pull request #11873:
URL: https://github.com/apache/arrow/pull/11873#issuecomment-989978046
Hi @jorisvandenbossche! I added the changes you suggested and all the checks are passing. Could you take another look? Thank you :D
--
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 #11873: ARROW-14242: Exposing the correct `indent` paramenter in `to_string`
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11873:
URL: https://github.com/apache/arrow/pull/11873#issuecomment-987176697
--
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 change in pull request #11873: ARROW-14242: Exposing the correct `indent` paramenter in `to_string`
Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on a change in pull request #11873:
URL: https://github.com/apache/arrow/pull/11873#discussion_r764722495
##########
File path: python/pyarrow/array.pxi
##########
@@ -1028,8 +1031,10 @@ cdef class Array(_PandasConvertible):
PrettyPrintOptions options
with nogil:
- options = PrettyPrintOptions(indent, window)
+ options = PrettyPrintOptions(top_level_indent, window)
options.skip_new_lines = skip_new_lines
+ if indent is not 0:
+ options.indent_size = indent
Review comment:
Actually, since this is a new keyword: we probably want to follow the C++ default (which is also what is used now), and so use a default of 2 here in Python as well for the `indent` keyword
##########
File path: python/pyarrow/array.pxi
##########
@@ -1028,8 +1031,10 @@ cdef class Array(_PandasConvertible):
PrettyPrintOptions options
with nogil:
- options = PrettyPrintOptions(indent, window)
+ options = PrettyPrintOptions(top_level_indent, window)
options.skip_new_lines = skip_new_lines
+ if indent is not 0:
+ options.indent_size = indent
Review comment:
We might want to set this always? (instead of only when it is not 0?) The default in C++ is 2 and not 0, so we will need to override that for 0 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 edited a comment on pull request #11873: ARROW-14242: Exposing the correct `indent` paramenter in `to_string`
Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #11873:
URL: https://github.com/apache/arrow/pull/11873#issuecomment-990057689
Benchmark runs are scheduled for baseline = b15a2f39659894f723e9def635cae4abdc2f02b9 and contender = 3ef40322dff954ecfb8754b394c2ca4de24a838b. 3ef40322dff954ecfb8754b394c2ca4de24a838b 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/f743234a6e6b4397bd39a2521ea244d8...60fe7118140240d29cf845fb2033f859/)
[Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/3d63624b2c1d4276890e755d1894da62...feb0dbb1732046bcad2af3e816e98e59/)
[Finished :arrow_down:0.49% :arrow_up:0.0%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/5d161ff536a649a2abc659d89967606b...fe1bb3495aa24ea6b8909a52e7ab4846/)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. Runs only benchmarks with cloud = True
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] ursabot edited a comment on pull request #11873: ARROW-14242: Exposing the correct `indent` paramenter in `to_string`
Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #11873:
URL: https://github.com/apache/arrow/pull/11873#issuecomment-990057689
Benchmark runs are scheduled for baseline = b15a2f39659894f723e9def635cae4abdc2f02b9 and contender = 3ef40322dff954ecfb8754b394c2ca4de24a838b. 3ef40322dff954ecfb8754b394c2ca4de24a838b 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/f743234a6e6b4397bd39a2521ea244d8...60fe7118140240d29cf845fb2033f859/)
[Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/3d63624b2c1d4276890e755d1894da62...feb0dbb1732046bcad2af3e816e98e59/)
[Scheduled] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/5d161ff536a649a2abc659d89967606b...fe1bb3495aa24ea6b8909a52e7ab4846/)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. Runs only benchmarks with cloud = True
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] jorisvandenbossche closed pull request #11873: ARROW-14242: Exposing the correct `indent` paramenter in `to_string`
Posted by GitBox <gi...@apache.org>.
jorisvandenbossche closed pull request #11873:
URL: https://github.com/apache/arrow/pull/11873
--
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 edited a comment on pull request #11873: ARROW-14242: Exposing the correct `indent` paramenter in `to_string`
Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #11873:
URL: https://github.com/apache/arrow/pull/11873#issuecomment-990057689
Benchmark runs are scheduled for baseline = b15a2f39659894f723e9def635cae4abdc2f02b9 and contender = 3ef40322dff954ecfb8754b394c2ca4de24a838b. 3ef40322dff954ecfb8754b394c2ca4de24a838b 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/f743234a6e6b4397bd39a2521ea244d8...60fe7118140240d29cf845fb2033f859/)
[Failed :arrow_down:0.45% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/3d63624b2c1d4276890e755d1894da62...feb0dbb1732046bcad2af3e816e98e59/)
[Finished :arrow_down:0.49% :arrow_up:0.0%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/5d161ff536a649a2abc659d89967606b...fe1bb3495aa24ea6b8909a52e7ab4846/)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. Runs only benchmarks with cloud = True
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] amol- commented on a change in pull request #11873: ARROW-14242: Exposing the correct `indent` paramenter in `to_string`
Posted by GitBox <gi...@apache.org>.
amol- commented on a change in pull request #11873:
URL: https://github.com/apache/arrow/pull/11873#discussion_r766598429
##########
File path: python/pyarrow/tests/test_array.py
##########
@@ -108,6 +108,22 @@ def test_long_array_format():
assert result == expected
+def test_indented_string_format():
+ arr = pa.array(['', None, 'foo'])
+ result = arr.to_string(indent=1)
+ expected = '[\n "",\n null,\n "foo"\n]'
Review comment:
I suggest we use multinline strings for the expectation, it's a bit easer to read for people getting to this 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] marlenezw commented on a change in pull request #11873: ARROW-14242: Exposing the correct `indent` paramenter in `to_string`
Posted by GitBox <gi...@apache.org>.
marlenezw commented on a change in pull request #11873:
URL: https://github.com/apache/arrow/pull/11873#discussion_r765048809
##########
File path: python/pyarrow/array.pxi
##########
@@ -1028,8 +1031,10 @@ cdef class Array(_PandasConvertible):
PrettyPrintOptions options
with nogil:
- options = PrettyPrintOptions(indent, window)
+ options = PrettyPrintOptions(top_level_indent, window)
options.skip_new_lines = skip_new_lines
+ if indent is not 0:
+ options.indent_size = indent
Review comment:
That sounds good! I'll set the default as 2 for the moment!
--
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] AlenkaF edited a comment on pull request #11873: ARROW-14242: Exposing the correct `indent` paramenter in `to_string`
Posted by GitBox <gi...@apache.org>.
AlenkaF edited a comment on pull request #11873:
URL: https://github.com/apache/arrow/pull/11873#issuecomment-987647700
Thank you Marlene!
The PR looks good. I do have some suggestions for improvements.
The CI is giving errors for tests using `to_string()` in `test_array.py`. I suggest to run the tests locally to see what has happened due to the change in the code and make the necessary corrections.
You mentioned you will add a test also, you could then add `indent` and `top_level_indent` specified to be different than 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] github-actions[bot] commented on pull request #11873: Exposing the `indent_size` paramenter in cython
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11873:
URL: https://github.com/apache/arrow/pull/11873#issuecomment-987049337
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
Thanks for opening a pull request!
If this is not a [minor PR](https://github.com/apache/arrow/blob/master/CONTRIBUTING.md#Minor-Fixes). Could you open an issue for this pull request on JIRA? https://issues.apache.org/jira/browse/ARROW
Opening JIRAs ahead of time contributes to the [Openness](http://theapacheway.com/open/#:~:text=Openness%20allows%20new%20users%20the,must%20happen%20in%20the%20open.) of the Apache Arrow project.
Then could you also rename pull request title in the following format?
ARROW-${JIRA_ID}: [${COMPONENT}] ${SUMMARY}
or
MINOR: [${COMPONENT}] ${SUMMARY}
See also:
* [Other pull requests](https://github.com/apache/arrow/pulls/)
* [Contribution Guidelines - How to contribute patches](https://arrow.apache.org/docs/developers/contributing.html#how-to-contribute-patches)
--
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] AlenkaF commented on pull request #11873: ARROW-14242: Exposing the correct `indent` paramenter in `to_string`
Posted by GitBox <gi...@apache.org>.
AlenkaF commented on pull request #11873:
URL: https://github.com/apache/arrow/pull/11873#issuecomment-987647700
Thank you Marlene!
The PR looks good. I do have some suggestions for improvements.
The CI is giving errors for tests using `to_string()` in `test_array.py`. I suggest to run the tests locally to see what has happened due to the change in the code and make the necessary corrections.
You could also add a test with `indent` and `top_level_indent` specified to be different than 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] ursabot commented on pull request #11873: ARROW-14242: Exposing the correct `indent` paramenter in `to_string`
Posted by GitBox <gi...@apache.org>.
ursabot commented on pull request #11873:
URL: https://github.com/apache/arrow/pull/11873#issuecomment-990057689
Benchmark runs are scheduled for baseline = b15a2f39659894f723e9def635cae4abdc2f02b9 and contender = 3ef40322dff954ecfb8754b394c2ca4de24a838b. 3ef40322dff954ecfb8754b394c2ca4de24a838b is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Scheduled] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/f743234a6e6b4397bd39a2521ea244d8...60fe7118140240d29cf845fb2033f859/)
[Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/3d63624b2c1d4276890e755d1894da62...feb0dbb1732046bcad2af3e816e98e59/)
[Scheduled] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/5d161ff536a649a2abc659d89967606b...fe1bb3495aa24ea6b8909a52e7ab4846/)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. Runs only benchmarks with cloud = True
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