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

Support DuckDBPyRelation kernel computations #37

Conversation

MarcSkovMadsen
Copy link
Collaborator

@MarcSkovMadsen MarcSkovMadsen commented Nov 10, 2024

Continues from #22. So review that one first.

Addresses #10 by adding kernel_computation=True support for duckdb.duckdb.DuckDBPyRelation objects supported by narwhals.

@MarcSkovMadsen
Copy link
Collaborator Author

MarcSkovMadsen commented Nov 10, 2024

I first tried to see if I could use the existing pygwalker database Connector class. But this needs a connectionstring and I simply could not figure out how to get it if I only have access to the relation because the connection is created by the user outside of "my control". I've asked for help in duckdb/duckdb#14768.

The current approach tries to implement a custom Connector class. The key point is to be able to execute subqueries on existing relations. It can actually be done in them most simple case but not more advanced cases (see added tests). I've asked for help in duckdb/duckdb#14772.

An alternative approach would be to see if I could use the BaseDataFrameParser. But as far as I can see I will run into the same problems as I have now.

Any one is free to take a look. I'm currently stuck and out of ideas.

@MarcSkovMadsen MarcSkovMadsen changed the title Enhancement/support duckdb relation Enhancement/support DuckDBPyRelation kernel computations Nov 10, 2024
@MarcSkovMadsen MarcSkovMadsen changed the title Enhancement/support DuckDBPyRelation kernel computations Support DuckDBPyRelation kernel computations Nov 10, 2024
@MarcSkovMadsen MarcSkovMadsen marked this pull request as ready for review November 10, 2024 09:13
@MarcSkovMadsen
Copy link
Collaborator Author

I managed to work around the problem by requiring the users register their duckdb connections. One day we might figure out how to avoid this.

@MarcSkovMadsen
Copy link
Collaborator Author

Found the .query argument which solves the issue.

@philippjfr philippjfr merged commit 30a9ed4 into panel-extensions:main Nov 10, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants