Skip to content
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(export): implement the Column.to_list() API #10498

Merged
merged 3 commits into from
Nov 18, 2024

Conversation

deepyaman
Copy link
Contributor

@deepyaman deepyaman commented Nov 15, 2024

Description of changes

Implement Column.to_list() convenience method to avoid having to do Column.to_pyarrow().to_pylist() manually.

Issues closed

@github-actions github-actions bot added the tests Issues or PRs related to tests label Nov 15, 2024
@deepyaman deepyaman marked this pull request as ready for review November 15, 2024 07:10
@deepyaman
Copy link
Contributor Author

deepyaman commented Nov 15, 2024

PySpark 3.3 issue is unrelated.

>>> t.bill_length_mm.to_list()
[39.1, 39.5, 40.3, None, 36.7]
"""
return self.to_pyarrow().to_pylist()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The kwargs need to be passed through to to_pyarrow here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, true, and docstring should say same as kwargs for to_pyarrow, too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wonder why the test is passing...

I'll take a look.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wonder why the test is passing...

The test logic was wrong; fixed in fe03b1d.

Parameters
----------
kwargs
Same as keyword arguments to [`to_pyarrow`](#ibis.expr.types.core.Expr.to_pyarrow)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is technically a broken link, since the docs aren't published... but so is

[`execute`]((#ibis.expr.types.core.Expr.execute)

used elsewhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if want to start publishing the docs so the links work? A linkchecker might be nice at some point. :D

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a link checker -- maybe it doesn't work on quartodoc-generated links? But yeah, lychee is the link checker and there's a checklinks rule in the justfile, so I dunno.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, interesting, we do publish Column docs, but not inherited methods, so we're missing out on things like to_pyarrow.

Well, that's a separate concern

Copy link
Member

@gforsyth gforsyth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@gforsyth gforsyth merged commit d11a292 into ibis-project:main Nov 18, 2024
76 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Issues or PRs related to tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat(ir): allow the creation of lists from columns
2 participants