westonpace commented on a change in pull request #10118:
URL: https://github.com/apache/arrow/pull/10118#discussion_r633123955
##########
File path: python/pyarrow/_dataset.pyx
##########
@@ -2746,6 +2746,10 @@ cdef class Scanner(_Weakrefable):
use_threads : bool, default True
If enabled, then maximum parallelism will be used determined by
the number of available CPU cores.
+ use_async : bool, default False
+ If enabled, the an async scanner will be used that should offer
+ better performance with high-latency/highly-parallel filesystems
+ (e.g. S3)
Review comment:
I updated all the tests so that they are parameterized on
use_threads/use_async. This helped identify the spots in _populate_builder,
etc. where I had missed passing the flag through.
##########
File path: python/pyarrow/_dataset.pyx
##########
@@ -2746,6 +2746,10 @@ cdef class Scanner(_Weakrefable):
use_threads : bool, default True
If enabled, then maximum parallelism will be used determined by
the number of available CPU cores.
+ use_async : bool, default False
+ If enabled, the an async scanner will be used that should offer
+ better performance with high-latency/highly-parallel filesystems
+ (e.g. S3)
Review comment:
I do think we might want to reduce the number of places we pass
parameters around but this was a problem before use_async (e.g. there are a lot
of places in the docs where we say "See scan method parameters
documentation."). That's not for this PR though and style-wise it might not be
so bad so python can just forward kwargs but when it dips into cython it seems
we need to materialized all the kwargs.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
