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

Circular import when loading external backends #2827

Closed
datapythonista opened this issue Jun 17, 2021 · 3 comments · Fixed by #2833
Closed

Circular import when loading external backends #2827

datapythonista opened this issue Jun 17, 2021 · 3 comments · Fixed by #2833
Labels
refactor Issues or PRs related to refactoring the codebase

Comments

@datapythonista
Copy link
Contributor

Looks like we've got a problem I haven't anticipated (and not sure why I didn't face this before while updating the OmniSci backend). We're currently loading all the backend found on entrypoints on import ibis. But backends also need to import ibis, so, there is a circular import when loading external backends.

The obvious solution would be to implement __getattr__ for the ibis module. So, on ibis.sqlite the backend would be loaded lazily. The problem with that is shown in the next code and there is #2671 for it:

import ibis

ibis.options.impala.temp_db = 'foo'

client = ibis.impala.connect(...)

We need to load the imapala backend on import ibis, if we do in ibis.impala.connect(...) the impala options are not defined, and ibis.options.impala.temp_db = ... fails. Loading the backend on options could be an option but seems too hacky to me.

So, in my opinion, we should take care of #2671 before the release (that would involve breaking changes, not just raising a FutureWarning). And I'd also implement #2532 which completely solves the problem, and backends would only be loaded after they are needed by an ibis.connect(...) call.

@jreback does this make sense to you? Any other idea?

@datapythonista datapythonista added the refactor Issues or PRs related to refactoring the codebase label Jun 17, 2021
@jreback
Copy link
Contributor

jreback commented Jun 17, 2021

we need to be able to set options w/o the connection parameter, that's sort of the entire point. I think loading on the options is ok here and would solve the problem.

I am not sure we want to break the world before 2.0

@gerrymanoim
Copy link
Contributor

I think I'm running into this same issue trying to path ibis_bigquery to work with the most recent ibis refactor. Very rough WIP where I was just trying to get tests running: gerrymanoim/ibis-bigquery@d98d812.

I end up in weird half initialized ibis import errors.

@datapythonista
Copy link
Contributor Author

I'm working on it, with the approach that Jeff mentioned. I should have a PR ready later today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Issues or PRs related to refactoring the codebase
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants