-
Notifications
You must be signed in to change notification settings - Fork 15
π§βπ» Support import of model classes before connecting to an instance #2825
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
Conversation
β¦nstance Adds support for importing lamindb model classes before running `ln.connect(...)` via lazy import proxies for all objects that are only available post connect. This plays much nicer with modern Python code style, where all imports are on the top of the file. After the user connects to an instance, instantiating the proxy classes returns instances of the corresponding models and calling classmethods on the proxy delegates to the model class.
The failing tests are currently due to my code changes. I'll debug what I am missing. |
β¦ule reload lamindb reloads imported modules on connect... and there seems to be an issue with from imports for the lazily imported submodules. Moving to absolute imports seems to restore the intended behavior.
some notes for next week It should probably be revisited why importlib.reload is needed at all in If it's required and can't be avoided it might be useful to adopt a ruff style for imports that prevents named from imports from within lamindb. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2825 +/- ##
==========================================
- Coverage 91.84% 91.21% -0.63%
==========================================
Files 69 66 -3
Lines 10807 9315 -1492
==========================================
- Hits 9926 8497 -1429
+ Misses 881 818 -63 β View full report in Codecov by Sentry. π New features to boost your workflow:
|
Deployment URL: https://473199ba.lamindb.pages.dev |
Closing in favour of #2849 |
Hi π
This adds support for importing lamindb model classes before running
ln.connect(...)
via lazy import proxies for all objects that are only available after connect. This plays much nicer with modern Python code style, where all imports are on the top of the file. After the user connects to an instance, instantiating the proxy classes returns instances of the corresponding models and calling classmethods on the proxy delegates to the model class.Notes
This needs tests, since I would bet that there are some edge cases where it breaks down. My guess is that auto-completion in jupyter might behave differently, but it needs to be checked.
I also noticed that
lamindb
imports a lot of submodules eagerly once connect happens, and I am not sure why you'd want that. I wonder if this enabled a better experience for users in jupyter notebooks and that's why you did it that way. It would be great to understand the reasoning behind it.In addition I changed the submodule import behavior in this PR slightly, and made the import of submodules "base", "errors", and "setup" also lazy (the three submodules that are eagerly imported before connect). I also made the module level
__getattr__
actually error with the correct Exception in case a user wants to access something that doesn't exists. For exampleimport lamindb as ln; ln.blah
will now correctly raise anAttributeError
.I'll continue with this a bit next week.