-
Notifications
You must be signed in to change notification settings - Fork 603
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(api): add to_pandas_batches
#7123
Conversation
7c8766e
to
45001a7
Compare
Added some more tests, fixed up issues. |
45001a7
to
7c23aa9
Compare
7c23aa9
to
08bf5bd
Compare
ibis/backends/snowflake/__init__.py
Outdated
params: Mapping[ir.Scalar, Any] | None = None, | ||
limit: int | str | None = None, | ||
**_: Any, | ||
) -> pa.ipc.RecordBatchReader: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incorrect type annotation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
ibis/expr/types/core.py
Outdated
params: Mapping[ir.Value, Any] | None = None, | ||
chunk_size: int = 1_000_000, | ||
**kwargs: Any, | ||
) -> pa.ipc.RecordBatchReader: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
ibis/backends/base/__init__.py
Outdated
schema = expr.schema() | ||
yield from ( | ||
orig_expr.__pandas_result__( | ||
self.pandas_converter.convert_table(batch.to_pandas(), schema) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIUC given this default definition, the PandasData
converter should work for dask as well, since it's always working on direct pandas data. No need for the pandas_converter
attribute at all, can just use PandasData
directly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, removed the attribute.
ibis/backends/base/__init__.py
Outdated
@@ -223,6 +224,8 @@ def _ipython_key_completions_(self) -> list[str]: | |||
|
|||
|
|||
class _FileIOHandler: | |||
pandas_converter = PandasData |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Slight preference for this to be a private attribute if we keep this, just so it doesn't show up in tab-completion for users.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the attribute entirely.
Clouds are passing:
|
20770a4
to
0b6f637
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
documentation nits, but this looks good to me
Clouds are good after the most recent commit:
|
Co-authored-by: Gil Forsyth <[email protected]>
924b3d8
to
f8432d7
Compare
Adds a
to_pandas_batches
for returning an iterator of pandas DataFrames from expression execution. Closes #6805.