You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "spenczar (via GitHub)" <gi...@apache.org> on 2023/08/22 23:31:09 UTC
[GitHub] [arrow] spenczar opened a new pull request, #37319: GH-37318: [Python]: Optimize combine_chunks when there is only one chunk
spenczar opened a new pull request, #37319:
URL: https://github.com/apache/arrow/pull/37319
### Rationale for this change
The associated issue explains the rationale.
I'd love to add benchmarks, but don't really know how; Arrow's benchmark system is pretty daunting for adding a pure-python change.
### What changes are included in this PR?
Only a Pyarrow change for combine_chunks to check the number of chunks and return the only chunk if there is only one.
### Are these changes tested?
Yes, no extra tests
### 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-37318: [Python]: Optimize combine_chunks when there is only one chunk [arrow]
Posted by "jorisvandenbossche (via GitHub)" <gi...@apache.org>.
jorisvandenbossche commented on PR #37319:
URL: https://github.com/apache/arrow/pull/37319#issuecomment-1748816009
Nothing, just pinging like you did! Sorry for the slow follow-up.
I was planning to merge it since I already approved it earlier, but then I thought of the following potential issue: one side-effect of this optimization is that this no longer copies the single-chunk case, but therefore also no longer reduces the memory in case this one chunk consists of a sliced array. See https://github.com/apache/arrow/issues/37878 for some context.
So the concat call can actually also have a beneficial side effect .. (but it's difficult to know which optimization is most important depending on the use case)
--
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-37318: [Python]: Optimize combine_chunks when there is only one chunk [arrow]
Posted by "spenczar (via GitHub)" <gi...@apache.org>.
spenczar closed pull request #37319: GH-37318: [Python]: Optimize combine_chunks when there is only one chunk
URL: https://github.com/apache/arrow/pull/37319
--
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-37318: [Python]: Optimize combine_chunks when there is only one chunk [arrow]
Posted by "spenczar (via GitHub)" <gi...@apache.org>.
spenczar commented on PR #37319:
URL: https://github.com/apache/arrow/pull/37319#issuecomment-1749502887
I don't think it's reasonable to say that undocumented side effects of `combine_chunks` (from a separate function it happens to call!) are part of its API. That just implies too much surface area for the entire Arrow project and makes improvement too hard. Perhaps for optimization, explicitness is more important than implicitness, and users should call `concat_arrays` upon the chunks if they want concat's side effects to occur.
Alternatively, we could have a new method (`ChunkedArray.to_array()` perhaps) which returns a single array but makes no promises about copying. The current thing doesn't make any promises either, but a new method would certainly be API-compatible. I think it would be difficult to write the API docs to explain the distinction between that new method and to_array.
--
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] spenczar commented on pull request #37319: GH-37318: [Python]: Optimize combine_chunks when there is only one chunk
Posted by "spenczar (via GitHub)" <gi...@apache.org>.
spenczar commented on PR #37319:
URL: https://github.com/apache/arrow/pull/37319#issuecomment-1737723979
Is there anything I can do to ensure this gets incorporated in the next release?
--
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 #37319: GH-37318: [Python]: Optimize combine_chunks when there is only one chunk
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #37319:
URL: https://github.com/apache/arrow/pull/37319#issuecomment-1689054222
:warning: GitHub issue #37318 **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
[GitHub] [arrow] jorisvandenbossche commented on pull request #37319: GH-37318: [Python]: Optimize combine_chunks when there is only one chunk
Posted by "jorisvandenbossche (via GitHub)" <gi...@apache.org>.
jorisvandenbossche commented on PR #37319:
URL: https://github.com/apache/arrow/pull/37319#issuecomment-1689809879
> I'd love to add benchmarks, but don't really know how; Arrow's benchmark system is pretty daunting for adding a pure-python change.
Yeah, we don't have much python benchmarks at the moment (the C++ benchmark setup isn't relevant here, since this is only for pyarrow). We do have some old ASVs at https://github.com/apache/arrow/tree/main/python/benchmarks, where you could add one. But at the moment we are not regularly running those benchmarks, so given that situation, that's certainly not required for this PR I would say.
--
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-37318: [Python]: Optimize combine_chunks when there is only one chunk [arrow]
Posted by "spenczar (via GitHub)" <gi...@apache.org>.
spenczar commented on PR #37319:
URL: https://github.com/apache/arrow/pull/37319#issuecomment-3324657962
Cleaning up old un-merged PRs in my queue. I still think this was a good idea but it appears to have no movement.
--
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