You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "Fokko (via GitHub)" <gi...@apache.org> on 2023/02/09 12:40:32 UTC
[GitHub] [arrow] Fokko opened a new pull request, #34099: GH-34098: [Python][Docs] Fix dataset docstring
Fokko opened a new pull request, #34099:
URL: https://github.com/apache/arrow/pull/34099
Many Dataset.from_{table,fragment,batches}
don't have a docstring, or they refer to
`Scanner.from_table`. It would be better
to plug in the actual docstring in there.
<!--
Thanks for opening a pull request!
If this is your first pull request you can find detailed information on how
to contribute here:
* [New Contributor's Guide](https://arrow.apache.org/docs/dev/developers/guide/step_by_step/pr_lifecycle.html#reviews-and-merge-of-the-pull-request)
* [Contributing Overview](https://arrow.apache.org/docs/dev/developers/overview.html)
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 GitHub? https://github.com/apache/arrow/issues/new/choose
Opening GitHub issues 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 the pull request title in the following format?
GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}
or
MINOR: [${COMPONENT}] ${SUMMARY}
In the case of PARQUET issues on JIRA the title also supports:
PARQUET-${JIRA_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}
-->
### Rationale for this change
<!--
Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.
-->
### What changes are included in this PR?
<!--
There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.
-->
### Are these changes tested?
<!--
We typically require tests for all PRs in order to:
1. Prevent the code from being accidentally broken by subsequent changes
2. Serve as another way to document the expected behavior of the code
If tests are not included in your PR, please explain why (for example, are they covered by existing tests)?
-->
### Are there any user-facing changes?
<!--
If there are user-facing changes then we may require documentation to be updated before approving the PR.
-->
<!--
If there are any breaking changes to public APIs, please uncomment the line below and explain which changes are breaking.
-->
<!-- **This PR includes breaking changes to public APIs.** -->
<!--
Please uncomment the line below (and provide explanation) if the changes fix either (a) a security vulnerability, (b) a bug that caused incorrect or invalid data to be produced, or (c) a bug that causes a crash (even when the API contract is upheld). We use this to highlight fixes to issues that may affect users without their knowledge. For this reason, fixing bugs that cause errors don't count, since those are usually obvious.
-->
<!-- **This PR contains a "Critical Fix".** -->
--
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] Fokko commented on pull request #34099: GH-34098: [Python][Docs] Fix dataset docstring
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on PR #34099:
URL: https://github.com/apache/arrow/pull/34099#issuecomment-1430423994
```
==================================== ERRORS ====================================
________________________ ERROR collecting test session _________________________
opt/conda/envs/arrow/lib/python3.8/importlib/__init__.py:127: in import_module
return _bootstrap._gcd_import(name[level:], package, level)
<frozen importlib._bootstrap>:1014: in _gcd_import
???
<frozen importlib._bootstrap>:991: in _find_and_load
???
<frozen importlib._bootstrap>:975: in _find_and_load_unlocked
???
<frozen importlib._bootstrap>:671: in _load_unlocked
???
opt/conda/envs/arrow/lib/python3.8/site-packages/_pytest/assertion/rewrite.py:168: in exec_module
exec(co, module.__dict__)
opt/conda/envs/arrow/lib/python3.8/site-packages/pyarrow/conftest.py:103: in <module>
import pyarrow.dataset # noqa
opt/conda/envs/arrow/lib/python3.8/site-packages/pyarrow/dataset.py:23: in <module>
from pyarrow._dataset import ( # noqa
pyarrow/_dataset.pyx:561: in init pyarrow._dataset
???
E AttributeError: attribute '__doc__' of 'method_descriptor' objects is not writable
!!!!!!!!!!!!!!!!!!!! Interrupted: 1 error during collection !!!!!!!!!!!!!!!!!!!!
=============================== 1 error in 0.95s ===============================
```
Line 561:
```python
Dataset.scanner.__doc__ = Dataset.scanner.__doc__.format(_scanner_arguments_doc)
```
--
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] Fokko commented on a diff in pull request #34099: GH-34098: [Python][Docs] Fix dataset docstring
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #34099:
URL: https://github.com/apache/arrow/pull/34099#discussion_r1115331032
##########
python/pyarrow/_dataset.pyx:
##########
@@ -348,63 +479,297 @@ cdef class Dataset(_Weakrefable):
Parameters
----------
- **kwargs : dict, optional
- Arguments for `Scanner.from_dataset`.
+ columns : list of str, default None
+ The columns to project. This can be a list of column names to
+ include (order and duplicates will be preserved), or a dictionary
+ with {new_column_name: expression} values for more advanced
+ projections.
+
+ The list of columns or expressions may use the special fields
+ `__batch_index` (the index of the batch within the fragment),
+ `__fragment_index` (the index of the fragment within the dataset),
+ `__last_in_fragment` (whether the batch is last in fragment), and
+ `__filename` (the name of the source file or a description of the
+ source fragment).
+
+ The columns will be passed down to Datasets and corresponding data
+ fragments to avoid loading, copying, and deserializing columns
+ that will not be required further down the compute chain.
+ By default all of the available columns are projected. Raises
+ an exception if any of the referenced column names does not exist
+ in the dataset's Schema.
+ filter : Expression, default None
+ Scan will return only the rows matching the filter.
+ If possible the predicate will be pushed down to exploit the
+ partition information or internal metadata found in the data
+ source, e.g. Parquet statistics. Otherwise filters the loaded
+ RecordBatches before yielding them.
+ batch_size : int, default 128Ki
+ The maximum row count for scanned record batches. If scanned
+ record batches are overflowing memory then this method can be
+ called to reduce their size.
+ batch_readahead : int, default 16
+ The number of batches to read ahead in a file. This might not work
+ for all file formats. Increasing this number will increase
+ RAM usage but could also improve IO utilization.
+ fragment_readahead : int, default 4
+ The number of files to read ahead. Increasing this number will increase
+ RAM usage but could also improve IO utilization.
+ fragment_scan_options : FragmentScanOptions, default None
+ Options specific to a particular scan and fragment type, which
+ can change between different scans of the same dataset.
+ use_threads : bool, default True
+ If enabled, then maximum parallelism will be used determined by
+ the number of available CPU cores.
+ memory_pool : MemoryPool, default None
+ For memory allocations, if required. If not specified, uses the
+ default pool.
Returns
-------
table : Table
"""
- return self.scanner(**kwargs).to_table()
-
- def take(self, object indices, **kwargs):
+ return self.scanner(
+ columns=columns,
+ filter=filter,
+ batch_size=batch_size,
+ batch_readahead=batch_readahead,
+ fragment_readahead=fragment_readahead,
+ fragment_scan_options=fragment_scan_options,
+ use_threads=use_threads,
+ memory_pool=memory_pool
+ ).to_table()
+
+ def take(self,
+ object indices,
+ object columns=None,
+ Expression filter=None,
+ int batch_size=_DEFAULT_BATCH_SIZE,
+ int batch_readahead=_DEFAULT_BATCH_READAHEAD,
+ int fragment_readahead=_DEFAULT_FRAGMENT_READAHEAD,
+ FragmentScanOptions fragment_scan_options=None,
+ bint use_threads=True,
+ MemoryPool memory_pool=None):
"""
Select rows of data by index.
Parameters
----------
indices : Array or array-like
indices of rows to select in the dataset.
- **kwargs : dict, optional
- See scanner() method for full parameter description.
+ columns : list of str, default None
+ The columns to project. This can be a list of column names to
+ include (order and duplicates will be preserved), or a dictionary
+ with {new_column_name: expression} values for more advanced
+ projections.
+
+ The list of columns or expressions may use the special fields
+ `__batch_index` (the index of the batch within the fragment),
+ `__fragment_index` (the index of the fragment within the dataset),
+ `__last_in_fragment` (whether the batch is last in fragment), and
+ `__filename` (the name of the source file or a description of the
+ source fragment).
+
+ The columns will be passed down to Datasets and corresponding data
+ fragments to avoid loading, copying, and deserializing columns
+ that will not be required further down the compute chain.
+ By default all of the available columns are projected. Raises
+ an exception if any of the referenced column names does not exist
+ in the dataset's Schema.
+ filter : Expression, default None
+ Scan will return only the rows matching the filter.
+ If possible the predicate will be pushed down to exploit the
+ partition information or internal metadata found in the data
+ source, e.g. Parquet statistics. Otherwise filters the loaded
+ RecordBatches before yielding them.
+ batch_size : int, default 128Ki
+ The maximum row count for scanned record batches. If scanned
+ record batches are overflowing memory then this method can be
+ called to reduce their size.
+ batch_readahead : int, default 16
+ The number of batches to read ahead in a file. This might not work
+ for all file formats. Increasing this number will increase
+ RAM usage but could also improve IO utilization.
+ fragment_readahead : int, default 4
+ The number of files to read ahead. Increasing this number will increase
+ RAM usage but could also improve IO utilization.
+ fragment_scan_options : FragmentScanOptions, default None
+ Options specific to a particular scan and fragment type, which
+ can change between different scans of the same dataset.
+ use_threads : bool, default True
+ If enabled, then maximum parallelism will be used determined by
+ the number of available CPU cores.
+ memory_pool : MemoryPool, default None
+ For memory allocations, if required. If not specified, uses the
+ default pool.
Returns
-------
table : Table
"""
- return self.scanner(**kwargs).take(indices)
-
- def head(self, int num_rows, **kwargs):
+ return self.scanner(
+ columns=columns,
+ filter=filter,
+ batch_size=batch_size,
+ batch_readahead=batch_readahead,
+ fragment_readahead=fragment_readahead,
+ fragment_scan_options=fragment_scan_options,
+ use_threads=use_threads,
+ memory_pool=memory_pool
+ ).take(indices)
+
+ def head(self,
+ int num_rows,
+ object columns=None,
+ Expression filter=None,
+ int batch_size=_DEFAULT_BATCH_SIZE,
+ int batch_readahead=_DEFAULT_BATCH_READAHEAD,
+ int fragment_readahead=_DEFAULT_FRAGMENT_READAHEAD,
+ FragmentScanOptions fragment_scan_options=None,
+ bint use_threads=True,
+ MemoryPool memory_pool=None):
"""
Load the first N rows of the dataset.
Parameters
----------
num_rows : int
The number of rows to load.
- **kwargs : dict, optional
- See scanner() method for full parameter description.
+ columns : list of str, default None
+ The columns to project. This can be a list of column names to
+ include (order and duplicates will be preserved), or a dictionary
+ with {new_column_name: expression} values for more advanced
+ projections.
+
+ The list of columns or expressions may use the special fields
+ `__batch_index` (the index of the batch within the fragment),
+ `__fragment_index` (the index of the fragment within the dataset),
+ `__last_in_fragment` (whether the batch is last in fragment), and
+ `__filename` (the name of the source file or a description of the
+ source fragment).
+
+ The columns will be passed down to Datasets and corresponding data
+ fragments to avoid loading, copying, and deserializing columns
+ that will not be required further down the compute chain.
+ By default all of the available columns are projected. Raises
+ an exception if any of the referenced column names does not exist
+ in the dataset's Schema.
+ filter : Expression, default None
+ Scan will return only the rows matching the filter.
+ If possible the predicate will be pushed down to exploit the
+ partition information or internal metadata found in the data
+ source, e.g. Parquet statistics. Otherwise filters the loaded
+ RecordBatches before yielding them.
+ batch_size : int, default 128Ki
+ The maximum row count for scanned record batches. If scanned
+ record batches are overflowing memory then this method can be
+ called to reduce their size.
+ batch_readahead : int, default 16
+ The number of batches to read ahead in a file. This might not work
+ for all file formats. Increasing this number will increase
+ RAM usage but could also improve IO utilization.
+ fragment_readahead : int, default 4
+ The number of files to read ahead. Increasing this number will increase
+ RAM usage but could also improve IO utilization.
+ fragment_scan_options : FragmentScanOptions, default None
+ Options specific to a particular scan and fragment type, which
+ can change between different scans of the same dataset.
+ use_threads : bool, default True
+ If enabled, then maximum parallelism will be used determined by
+ the number of available CPU cores.
+ memory_pool : MemoryPool, default None
+ For memory allocations, if required. If not specified, uses the
+ default pool.
Returns
-------
table : Table
"""
- return self.scanner(**kwargs).head(num_rows)
-
- def count_rows(self, **kwargs):
+ return self.scanner(
+ columns=columns,
+ filter=filter,
+ batch_size=batch_size,
+ batch_readahead=batch_readahead,
+ fragment_readahead=fragment_readahead,
+ fragment_scan_options=fragment_scan_options,
+ use_threads=use_threads,
+ memory_pool=memory_pool
+ ).head(num_rows)
+
+ def count_rows(self,
+ object columns=None,
Review Comment:
@jorisvandenbossche Sorry for the long wait, I just removed the `count` for `count_rows`. I think we're okay with maintaining the docstrings 👍🏻
--
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] Fokko commented on pull request #34099: GH-34098: [Python][Docs] Fix dataset docstring
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on PR #34099:
URL: https://github.com/apache/arrow/pull/34099#issuecomment-1430425817
I don't think that the decorator will work, it is also overwriting the `__doc__` attribute: https://github.com/pandas-dev/pandas/blob/7d545f0849b8502974d119684bef744382cb55be/pandas/util/_decorators.py#L383-L390
--
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 #34099: GH-34098: [Python][Docs] Fix dataset docstring
Posted by "jorisvandenbossche (via GitHub)" <gi...@apache.org>.
jorisvandenbossche commented on PR #34099:
URL: https://github.com/apache/arrow/pull/34099#issuecomment-1429755044
@Fokko Thanks for the PR! Generally it looks great, but in practice it seems the `.format(..)` might not work for docstrings? If I test out this branch, I get empty docstrings for eg `pyarrow.dataset.Dataset.to_batches`
In pandas, they use a `@doc` decorator that does this, I assume to overcome this limitation (https://github.com/pandas-dev/pandas/blob/7d545f0849b8502974d119684bef744382cb55be/pandas/util/_decorators.py#L340-L398, it can do a lot more than just filling in some parts, so it's more complicated than what we would need). But I don't know if this would work in cython code. This would be used like
```
class Scanner:
@doc(_scanner_arguments_doc)
def to_batches(..):
"""
Read the dataset as materialized record batches.
Parameters
----------
{0}
"""
```
Instead of calling the `.format` inline (and the `@doc` decorator basically does that under the hood)
In our own parquet module we use the simpler approach of afterwards assigning `__doc__`:
https://github.com/apache/arrow/blob/ddfa8eed9b188fcc7b38767d1858c2588c588f05/python/pyarrow/parquet/core.py#L3019-L3029
Similarly like that, you could also leave the docstrings as you updated them in this PR, but do the string interpolation as a next step:
```
class Scanner:
@doc(_scanner_arguments_doc)
def to_batches(..):
"""
Read the dataset as materialized record batches.
Parameters
----------
{0}
"""
...
Scanner.to_batches.__doc__ = Scanner.to_batches.__doc__.format(_scanner_arguments_doc)
```
--
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 #34099: GH-34098: [Python][Docs] Expand dataset method docstrings
Posted by "ursabot (via GitHub)" <gi...@apache.org>.
ursabot commented on PR #34099:
URL: https://github.com/apache/arrow/pull/34099#issuecomment-1448974351
Benchmark runs are scheduled for baseline = 0e752c8655685a0108c3eed3de27e7981db688c7 and contender = 61c9a749741fcfad5de5d0e1e7fe1ced1330928a. 61c9a749741fcfad5de5d0e1e7fe1ced1330928a 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/e04a671135f4478f8a752e1e15dd61ac...2b0f4c3530fc432cb4219aa07e28f8ff/)
[Finished :arrow_down:0.73% :arrow_up:0.0%] [test-mac-arm](https://conbench.ursa.dev/compare/runs/815b321156c44e51ac4cc628121818fc...ac48c710ba744321b97b7ee68ff01444/)
[Finished :arrow_down:0.26% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/a80b3097d965497c92faaaa8efdeb4eb...942fd3bc8c5c46ae9567c47426fa4e9d/)
[Finished :arrow_down:0.06% :arrow_up:0.1%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/38113cbf63794de6862a296fb9234c7d...f89873cbe0164f5fa9d75716f85aec4a/)
Buildkite builds:
[Finished] [`61c9a749` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/2447)
[Finished] [`61c9a749` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/2477)
[Finished] [`61c9a749` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/2444)
[Finished] [`61c9a749` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/2468)
[Finished] [`0e752c86` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/2446)
[Finished] [`0e752c86` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/2476)
[Finished] [`0e752c86` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/2443)
[Finished] [`0e752c86` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/2467)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
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 merged pull request #34099: GH-34098: [Python][Docs] Fix dataset docstring
Posted by "jorisvandenbossche (via GitHub)" <gi...@apache.org>.
jorisvandenbossche merged PR #34099:
URL: https://github.com/apache/arrow/pull/34099
--
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] Fokko commented on pull request #34099: GH-34098: [Python][Docs] Fix dataset docstring
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on PR #34099:
URL: https://github.com/apache/arrow/pull/34099#issuecomment-1447750709
@jorisvandenbossche any further thoughts on this?
--
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 #34099: GH-34098: [Python][Docs] Fix dataset docstring
Posted by "jorisvandenbossche (via GitHub)" <gi...@apache.org>.
jorisvandenbossche commented on PR #34099:
URL: https://github.com/apache/arrow/pull/34099#issuecomment-1433232408
Yes, certainly agree that some duplication is worth the improved usability of those docstrings.
I do wonder if we might want to go for some middle ground, and add it to the most used methods (like to_table, to_batches, and some others). With methods like `count_rows` and `head` you probably typically won't use those keywords, so in those methods referring to the docstring of another method might be good enough.
--
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] Fokko commented on a diff in pull request #34099: GH-34098: [Python][Docs] Fix dataset docstring
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #34099:
URL: https://github.com/apache/arrow/pull/34099#discussion_r1108710032
##########
python/pyarrow/_dataset.pyx:
##########
@@ -348,63 +479,297 @@ cdef class Dataset(_Weakrefable):
Parameters
----------
- **kwargs : dict, optional
- Arguments for `Scanner.from_dataset`.
+ columns : list of str, default None
+ The columns to project. This can be a list of column names to
+ include (order and duplicates will be preserved), or a dictionary
+ with {new_column_name: expression} values for more advanced
+ projections.
+
+ The list of columns or expressions may use the special fields
+ `__batch_index` (the index of the batch within the fragment),
+ `__fragment_index` (the index of the fragment within the dataset),
+ `__last_in_fragment` (whether the batch is last in fragment), and
+ `__filename` (the name of the source file or a description of the
+ source fragment).
+
+ The columns will be passed down to Datasets and corresponding data
+ fragments to avoid loading, copying, and deserializing columns
+ that will not be required further down the compute chain.
+ By default all of the available columns are projected. Raises
+ an exception if any of the referenced column names does not exist
+ in the dataset's Schema.
+ filter : Expression, default None
+ Scan will return only the rows matching the filter.
+ If possible the predicate will be pushed down to exploit the
+ partition information or internal metadata found in the data
+ source, e.g. Parquet statistics. Otherwise filters the loaded
+ RecordBatches before yielding them.
+ batch_size : int, default 128Ki
+ The maximum row count for scanned record batches. If scanned
+ record batches are overflowing memory then this method can be
+ called to reduce their size.
+ batch_readahead : int, default 16
+ The number of batches to read ahead in a file. This might not work
+ for all file formats. Increasing this number will increase
+ RAM usage but could also improve IO utilization.
+ fragment_readahead : int, default 4
+ The number of files to read ahead. Increasing this number will increase
+ RAM usage but could also improve IO utilization.
+ fragment_scan_options : FragmentScanOptions, default None
+ Options specific to a particular scan and fragment type, which
+ can change between different scans of the same dataset.
+ use_threads : bool, default True
+ If enabled, then maximum parallelism will be used determined by
+ the number of available CPU cores.
+ memory_pool : MemoryPool, default None
+ For memory allocations, if required. If not specified, uses the
+ default pool.
Returns
-------
table : Table
"""
- return self.scanner(**kwargs).to_table()
-
- def take(self, object indices, **kwargs):
+ return self.scanner(
+ columns=columns,
+ filter=filter,
+ batch_size=batch_size,
+ batch_readahead=batch_readahead,
+ fragment_readahead=fragment_readahead,
+ fragment_scan_options=fragment_scan_options,
+ use_threads=use_threads,
+ memory_pool=memory_pool
+ ).to_table()
+
+ def take(self,
+ object indices,
+ object columns=None,
+ Expression filter=None,
+ int batch_size=_DEFAULT_BATCH_SIZE,
+ int batch_readahead=_DEFAULT_BATCH_READAHEAD,
+ int fragment_readahead=_DEFAULT_FRAGMENT_READAHEAD,
+ FragmentScanOptions fragment_scan_options=None,
+ bint use_threads=True,
+ MemoryPool memory_pool=None):
"""
Select rows of data by index.
Parameters
----------
indices : Array or array-like
indices of rows to select in the dataset.
- **kwargs : dict, optional
- See scanner() method for full parameter description.
+ columns : list of str, default None
+ The columns to project. This can be a list of column names to
+ include (order and duplicates will be preserved), or a dictionary
+ with {new_column_name: expression} values for more advanced
+ projections.
+
+ The list of columns or expressions may use the special fields
+ `__batch_index` (the index of the batch within the fragment),
+ `__fragment_index` (the index of the fragment within the dataset),
+ `__last_in_fragment` (whether the batch is last in fragment), and
+ `__filename` (the name of the source file or a description of the
+ source fragment).
+
+ The columns will be passed down to Datasets and corresponding data
+ fragments to avoid loading, copying, and deserializing columns
+ that will not be required further down the compute chain.
+ By default all of the available columns are projected. Raises
+ an exception if any of the referenced column names does not exist
+ in the dataset's Schema.
+ filter : Expression, default None
+ Scan will return only the rows matching the filter.
+ If possible the predicate will be pushed down to exploit the
+ partition information or internal metadata found in the data
+ source, e.g. Parquet statistics. Otherwise filters the loaded
+ RecordBatches before yielding them.
+ batch_size : int, default 128Ki
+ The maximum row count for scanned record batches. If scanned
+ record batches are overflowing memory then this method can be
+ called to reduce their size.
+ batch_readahead : int, default 16
+ The number of batches to read ahead in a file. This might not work
+ for all file formats. Increasing this number will increase
+ RAM usage but could also improve IO utilization.
+ fragment_readahead : int, default 4
+ The number of files to read ahead. Increasing this number will increase
+ RAM usage but could also improve IO utilization.
+ fragment_scan_options : FragmentScanOptions, default None
+ Options specific to a particular scan and fragment type, which
+ can change between different scans of the same dataset.
+ use_threads : bool, default True
+ If enabled, then maximum parallelism will be used determined by
+ the number of available CPU cores.
+ memory_pool : MemoryPool, default None
+ For memory allocations, if required. If not specified, uses the
+ default pool.
Returns
-------
table : Table
"""
- return self.scanner(**kwargs).take(indices)
-
- def head(self, int num_rows, **kwargs):
+ return self.scanner(
+ columns=columns,
+ filter=filter,
+ batch_size=batch_size,
+ batch_readahead=batch_readahead,
+ fragment_readahead=fragment_readahead,
+ fragment_scan_options=fragment_scan_options,
+ use_threads=use_threads,
+ memory_pool=memory_pool
+ ).take(indices)
+
+ def head(self,
+ int num_rows,
+ object columns=None,
+ Expression filter=None,
+ int batch_size=_DEFAULT_BATCH_SIZE,
+ int batch_readahead=_DEFAULT_BATCH_READAHEAD,
+ int fragment_readahead=_DEFAULT_FRAGMENT_READAHEAD,
+ FragmentScanOptions fragment_scan_options=None,
+ bint use_threads=True,
+ MemoryPool memory_pool=None):
"""
Load the first N rows of the dataset.
Parameters
----------
num_rows : int
The number of rows to load.
- **kwargs : dict, optional
- See scanner() method for full parameter description.
+ columns : list of str, default None
+ The columns to project. This can be a list of column names to
+ include (order and duplicates will be preserved), or a dictionary
+ with {new_column_name: expression} values for more advanced
+ projections.
+
+ The list of columns or expressions may use the special fields
+ `__batch_index` (the index of the batch within the fragment),
+ `__fragment_index` (the index of the fragment within the dataset),
+ `__last_in_fragment` (whether the batch is last in fragment), and
+ `__filename` (the name of the source file or a description of the
+ source fragment).
+
+ The columns will be passed down to Datasets and corresponding data
+ fragments to avoid loading, copying, and deserializing columns
+ that will not be required further down the compute chain.
+ By default all of the available columns are projected. Raises
+ an exception if any of the referenced column names does not exist
+ in the dataset's Schema.
+ filter : Expression, default None
+ Scan will return only the rows matching the filter.
+ If possible the predicate will be pushed down to exploit the
+ partition information or internal metadata found in the data
+ source, e.g. Parquet statistics. Otherwise filters the loaded
+ RecordBatches before yielding them.
+ batch_size : int, default 128Ki
+ The maximum row count for scanned record batches. If scanned
+ record batches are overflowing memory then this method can be
+ called to reduce their size.
+ batch_readahead : int, default 16
+ The number of batches to read ahead in a file. This might not work
+ for all file formats. Increasing this number will increase
+ RAM usage but could also improve IO utilization.
+ fragment_readahead : int, default 4
+ The number of files to read ahead. Increasing this number will increase
+ RAM usage but could also improve IO utilization.
+ fragment_scan_options : FragmentScanOptions, default None
+ Options specific to a particular scan and fragment type, which
+ can change between different scans of the same dataset.
+ use_threads : bool, default True
+ If enabled, then maximum parallelism will be used determined by
+ the number of available CPU cores.
+ memory_pool : MemoryPool, default None
+ For memory allocations, if required. If not specified, uses the
+ default pool.
Returns
-------
table : Table
"""
- return self.scanner(**kwargs).head(num_rows)
-
- def count_rows(self, **kwargs):
+ return self.scanner(
+ columns=columns,
+ filter=filter,
+ batch_size=batch_size,
+ batch_readahead=batch_readahead,
+ fragment_readahead=fragment_readahead,
+ fragment_scan_options=fragment_scan_options,
+ use_threads=use_threads,
+ memory_pool=memory_pool
+ ).head(num_rows)
+
+ def count_rows(self,
+ object columns=None,
Review Comment:
I'd rather strip away the columns that don't make sense for a special operation. The problem is that the `*kwargs` also need to have a docstring, which would have a reference to another function, but then in PyCharm it cannot infer the arguments from the docstring (and you have to go back to the docs).
Columns make sense in a SQL case, where `count(col)` will only count the valid columns, instead of a `count(1)` like it is 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
[GitHub] [arrow] Fokko commented on a diff in pull request #34099: GH-34098: [Python][Docs] Fix dataset docstring
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #34099:
URL: https://github.com/apache/arrow/pull/34099#discussion_r1101413477
##########
python/pyarrow/_dataset.pyx:
##########
@@ -2393,53 +2434,6 @@ cdef class Scanner(_Weakrefable):
A scanner is the class that glues the scan tasks, data fragments and data
sources together.
-
- Parameters
Review Comment:
I removed these since the default constructor only accepts a pointer to another dataset, and doesn't accept these arguments.
--
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] Fokko commented on a diff in pull request #34099: GH-34098: [Python][Docs] Fix dataset docstring
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #34099:
URL: https://github.com/apache/arrow/pull/34099#discussion_r1101412505
##########
python/pyarrow/_dataset.pyx:
##########
@@ -2647,9 +2551,11 @@ cdef class Scanner(_Weakrefable):
@staticmethod
def from_batches(source, *, Schema schema=None, object columns=None,
Expression filter=None, int batch_size=_DEFAULT_BATCH_SIZE,
+ int batch_readahead=_DEFAULT_BATCH_READAHEAD,
Review Comment:
Added the missing arguments
--
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 #34099: GH-34098: [Python][Docs] Fix dataset docstring
Posted by "jorisvandenbossche (via GitHub)" <gi...@apache.org>.
jorisvandenbossche commented on PR #34099:
URL: https://github.com/apache/arrow/pull/34099#issuecomment-1433134263
Yes, so the issues with not being able to write to `__doc__` is a general CPython issue. Antoine actaully opened an issue about it (so I suppose we tried this before ..): https://github.com/python/cpython/issues/91309
I suppose just copying the docstring around as you did now is the best we can do ..
--
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 diff in pull request #34099: GH-34098: [Python][Docs] Fix dataset docstring
Posted by "jorisvandenbossche (via GitHub)" <gi...@apache.org>.
jorisvandenbossche commented on code in PR #34099:
URL: https://github.com/apache/arrow/pull/34099#discussion_r1108573671
##########
python/pyarrow/_dataset.pyx:
##########
@@ -348,63 +479,297 @@ cdef class Dataset(_Weakrefable):
Parameters
----------
- **kwargs : dict, optional
- Arguments for `Scanner.from_dataset`.
+ columns : list of str, default None
+ The columns to project. This can be a list of column names to
+ include (order and duplicates will be preserved), or a dictionary
+ with {new_column_name: expression} values for more advanced
+ projections.
+
+ The list of columns or expressions may use the special fields
+ `__batch_index` (the index of the batch within the fragment),
+ `__fragment_index` (the index of the fragment within the dataset),
+ `__last_in_fragment` (whether the batch is last in fragment), and
+ `__filename` (the name of the source file or a description of the
+ source fragment).
+
+ The columns will be passed down to Datasets and corresponding data
+ fragments to avoid loading, copying, and deserializing columns
+ that will not be required further down the compute chain.
+ By default all of the available columns are projected. Raises
+ an exception if any of the referenced column names does not exist
+ in the dataset's Schema.
+ filter : Expression, default None
+ Scan will return only the rows matching the filter.
+ If possible the predicate will be pushed down to exploit the
+ partition information or internal metadata found in the data
+ source, e.g. Parquet statistics. Otherwise filters the loaded
+ RecordBatches before yielding them.
+ batch_size : int, default 128Ki
+ The maximum row count for scanned record batches. If scanned
+ record batches are overflowing memory then this method can be
+ called to reduce their size.
+ batch_readahead : int, default 16
+ The number of batches to read ahead in a file. This might not work
+ for all file formats. Increasing this number will increase
+ RAM usage but could also improve IO utilization.
+ fragment_readahead : int, default 4
+ The number of files to read ahead. Increasing this number will increase
+ RAM usage but could also improve IO utilization.
+ fragment_scan_options : FragmentScanOptions, default None
+ Options specific to a particular scan and fragment type, which
+ can change between different scans of the same dataset.
+ use_threads : bool, default True
+ If enabled, then maximum parallelism will be used determined by
+ the number of available CPU cores.
+ memory_pool : MemoryPool, default None
+ For memory allocations, if required. If not specified, uses the
+ default pool.
Returns
-------
table : Table
"""
- return self.scanner(**kwargs).to_table()
-
- def take(self, object indices, **kwargs):
+ return self.scanner(
+ columns=columns,
+ filter=filter,
+ batch_size=batch_size,
+ batch_readahead=batch_readahead,
+ fragment_readahead=fragment_readahead,
+ fragment_scan_options=fragment_scan_options,
+ use_threads=use_threads,
+ memory_pool=memory_pool
+ ).to_table()
+
+ def take(self,
+ object indices,
+ object columns=None,
+ Expression filter=None,
+ int batch_size=_DEFAULT_BATCH_SIZE,
+ int batch_readahead=_DEFAULT_BATCH_READAHEAD,
+ int fragment_readahead=_DEFAULT_FRAGMENT_READAHEAD,
+ FragmentScanOptions fragment_scan_options=None,
+ bint use_threads=True,
+ MemoryPool memory_pool=None):
"""
Select rows of data by index.
Parameters
----------
indices : Array or array-like
indices of rows to select in the dataset.
- **kwargs : dict, optional
- See scanner() method for full parameter description.
+ columns : list of str, default None
+ The columns to project. This can be a list of column names to
+ include (order and duplicates will be preserved), or a dictionary
+ with {new_column_name: expression} values for more advanced
+ projections.
+
+ The list of columns or expressions may use the special fields
+ `__batch_index` (the index of the batch within the fragment),
+ `__fragment_index` (the index of the fragment within the dataset),
+ `__last_in_fragment` (whether the batch is last in fragment), and
+ `__filename` (the name of the source file or a description of the
+ source fragment).
+
+ The columns will be passed down to Datasets and corresponding data
+ fragments to avoid loading, copying, and deserializing columns
+ that will not be required further down the compute chain.
+ By default all of the available columns are projected. Raises
+ an exception if any of the referenced column names does not exist
+ in the dataset's Schema.
+ filter : Expression, default None
+ Scan will return only the rows matching the filter.
+ If possible the predicate will be pushed down to exploit the
+ partition information or internal metadata found in the data
+ source, e.g. Parquet statistics. Otherwise filters the loaded
+ RecordBatches before yielding them.
+ batch_size : int, default 128Ki
+ The maximum row count for scanned record batches. If scanned
+ record batches are overflowing memory then this method can be
+ called to reduce their size.
+ batch_readahead : int, default 16
+ The number of batches to read ahead in a file. This might not work
+ for all file formats. Increasing this number will increase
+ RAM usage but could also improve IO utilization.
+ fragment_readahead : int, default 4
+ The number of files to read ahead. Increasing this number will increase
+ RAM usage but could also improve IO utilization.
+ fragment_scan_options : FragmentScanOptions, default None
+ Options specific to a particular scan and fragment type, which
+ can change between different scans of the same dataset.
+ use_threads : bool, default True
+ If enabled, then maximum parallelism will be used determined by
+ the number of available CPU cores.
+ memory_pool : MemoryPool, default None
+ For memory allocations, if required. If not specified, uses the
+ default pool.
Returns
-------
table : Table
"""
- return self.scanner(**kwargs).take(indices)
-
- def head(self, int num_rows, **kwargs):
+ return self.scanner(
+ columns=columns,
+ filter=filter,
+ batch_size=batch_size,
+ batch_readahead=batch_readahead,
+ fragment_readahead=fragment_readahead,
+ fragment_scan_options=fragment_scan_options,
+ use_threads=use_threads,
+ memory_pool=memory_pool
+ ).take(indices)
+
+ def head(self,
+ int num_rows,
+ object columns=None,
+ Expression filter=None,
+ int batch_size=_DEFAULT_BATCH_SIZE,
+ int batch_readahead=_DEFAULT_BATCH_READAHEAD,
+ int fragment_readahead=_DEFAULT_FRAGMENT_READAHEAD,
+ FragmentScanOptions fragment_scan_options=None,
+ bint use_threads=True,
+ MemoryPool memory_pool=None):
"""
Load the first N rows of the dataset.
Parameters
----------
num_rows : int
The number of rows to load.
- **kwargs : dict, optional
- See scanner() method for full parameter description.
+ columns : list of str, default None
+ The columns to project. This can be a list of column names to
+ include (order and duplicates will be preserved), or a dictionary
+ with {new_column_name: expression} values for more advanced
+ projections.
+
+ The list of columns or expressions may use the special fields
+ `__batch_index` (the index of the batch within the fragment),
+ `__fragment_index` (the index of the fragment within the dataset),
+ `__last_in_fragment` (whether the batch is last in fragment), and
+ `__filename` (the name of the source file or a description of the
+ source fragment).
+
+ The columns will be passed down to Datasets and corresponding data
+ fragments to avoid loading, copying, and deserializing columns
+ that will not be required further down the compute chain.
+ By default all of the available columns are projected. Raises
+ an exception if any of the referenced column names does not exist
+ in the dataset's Schema.
+ filter : Expression, default None
+ Scan will return only the rows matching the filter.
+ If possible the predicate will be pushed down to exploit the
+ partition information or internal metadata found in the data
+ source, e.g. Parquet statistics. Otherwise filters the loaded
+ RecordBatches before yielding them.
+ batch_size : int, default 128Ki
+ The maximum row count for scanned record batches. If scanned
+ record batches are overflowing memory then this method can be
+ called to reduce their size.
+ batch_readahead : int, default 16
+ The number of batches to read ahead in a file. This might not work
+ for all file formats. Increasing this number will increase
+ RAM usage but could also improve IO utilization.
+ fragment_readahead : int, default 4
+ The number of files to read ahead. Increasing this number will increase
+ RAM usage but could also improve IO utilization.
+ fragment_scan_options : FragmentScanOptions, default None
+ Options specific to a particular scan and fragment type, which
+ can change between different scans of the same dataset.
+ use_threads : bool, default True
+ If enabled, then maximum parallelism will be used determined by
+ the number of available CPU cores.
+ memory_pool : MemoryPool, default None
+ For memory allocations, if required. If not specified, uses the
+ default pool.
Returns
-------
table : Table
"""
- return self.scanner(**kwargs).head(num_rows)
-
- def count_rows(self, **kwargs):
+ return self.scanner(
+ columns=columns,
+ filter=filter,
+ batch_size=batch_size,
+ batch_readahead=batch_readahead,
+ fragment_readahead=fragment_readahead,
+ fragment_scan_options=fragment_scan_options,
+ use_threads=use_threads,
+ memory_pool=memory_pool
+ ).head(num_rows)
+
+ def count_rows(self,
+ object columns=None,
Review Comment:
For count_rows, doing a column selection doesn't really make sense? (it shouldn't influence the result)
Maybe we can keep it in `**kwargs` to be sure, but at least we can maybe limit its section in the docstring here
(starting to make special cases of course makes it harder to maintain the repeated docstrings ...)
--
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] Fokko commented on pull request #34099: GH-34098: [Python][Docs] Fix dataset docstring
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on PR #34099:
URL: https://github.com/apache/arrow/pull/34099#issuecomment-1433148827
Ah, great catch on the cpython issue. I'm also not super happy about copying it, but I think updating shouldn't be the biggest issue. At least there are checks that make sure that the arguments are documented. I think it is worth copying the docstrings to make it more explicit, and therefore friendlier, to the end user.
--
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 #34099: GH-34098: [Python][Docs] Fix dataset docstring
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34099:
URL: https://github.com/apache/arrow/pull/34099#issuecomment-1424134023
:warning: GitHub issue #34098 **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] github-actions[bot] commented on pull request #34099: GH-34098: [Python][Docs] Fix dataset docstring
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34099:
URL: https://github.com/apache/arrow/pull/34099#issuecomment-1424133957
* Closes: #34098
--
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] Fokko commented on pull request #34099: GH-34098: [Python][Docs] Fix dataset docstring
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on PR #34099:
URL: https://github.com/apache/arrow/pull/34099#issuecomment-1430407730
@jorisvandenbossche Thanks for the pointers. I'm quite comfortable with Python, and the examples point to `.py` files. For some reason, this doesn't seem to work with the `.pyx` files. Let me take another stab at 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
[GitHub] [arrow] Fokko commented on pull request #34099: GH-34098: [Python][Docs] Expand dataset method docstrings
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on PR #34099:
URL: https://github.com/apache/arrow/pull/34099#issuecomment-1447795914
Awesome, thanks @jorisvandenbossche 🚀
--
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