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

refactor(register): remove deprecated register method #10545

Merged
merged 4 commits into from
Dec 17, 2024

Conversation

gforsyth
Copy link
Member

refactor(register): remove deprecated register method

BREAKING CHANGE: The deprecated register method has been removed.
Please use the file-specific read_* methods instead. For in-memory
objects, pass them to ibis.memtable or create_table.

Description of changes

The register method is removed everywhere. Test files have been renamed to
test_io.py instead of test_register.py, or they have been removed entirely
because they are duplicates of tests in test_client.py or elsewhere.

Issues closed

#10175

@github-actions github-actions bot added tests Issues or PRs related to tests pyspark The Apache PySpark backend datafusion The Apache DataFusion backend duckdb The DuckDB backend polars The polars backend labels Nov 29, 2024
BREAKING CHANGE: The deprecated `register` method has been removed.
Please use the file-specific `read_*` methods instead. For in-memory
objects, pass them to `ibis.memtable` or `create_table`.
@gforsyth
Copy link
Member Author

Hmm, so the datafusion backend has an option which is unique to that backend, where we allow a user to pass in a dictionary of {table_name: file_to_load} -- at connection creation, we hand each of these files off to register to load all of the files as tables.

I recognize the convenience here, but it's also against our general push towards requiring explicit table creation.

Do we rip that out, too? Or leave in _register as a private method in the datafusion backend to handle this?

@IndexSeek
Copy link
Member

Thank you for prepping this!

Do we rip that out, too? Or leave in _register as a private method in the datafusion backend to handle this?

That's tricky. I can't imagine it's used too widely, but I vote to leave it there if anyone uses that functionality with the DataFusion backend today.

Could we mark it deprecated and for the instead guide the user in creating these tables explicitly?

@gforsyth
Copy link
Member Author

gforsyth commented Dec 4, 2024

That's tricky. I can't imagine it's used too widely, but I vote to leave it there if anyone uses that functionality with the DataFusion backend today.

Could we mark it deprecated and for the instead guide the user in creating these tables explicitly?

Thanks, @IndexSeek ! This seemed like a good suggestion, so I've remove the public register method, but left the private one in along with deprecating its usage in loading files at connection time.

@cpcloud cpcloud added this to the 10.0 milestone Dec 17, 2024
Copy link
Member

@cpcloud cpcloud left a comment

Choose a reason for hiding this comment

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

🚢 it

@cpcloud cpcloud merged commit aa60584 into ibis-project:main Dec 17, 2024
75 checks passed
@gforsyth gforsyth deleted the remove_register branch December 17, 2024 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
datafusion The Apache DataFusion backend duckdb The DuckDB backend polars The polars backend pyspark The Apache PySpark backend tests Issues or PRs related to tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants