ENH Adds lazy module at the top level #29793
Conversation
There was a problem hiding this comment.
I think the added complexity/magic is minimal and the ability to do a single import sklearn line in an IPython session followed by code composing classes looked from different modules by qualnames is quite convenient.
So +1 on my side.
I am not sure about how to describe this in the changelog entry, but maybe this is better to introduce this with an example in the release highlights.
Before merging this PR, we might want to get more feedback than usual though, either by more reviews / votes or by validating it at the next monthly dev meeting.
EDIT: __getattr__ magic (even if quite straightforward in this case) and introducing multiple usage/import styles for sklearn make me ponder the above. So I am +0 instead of +1.
jeremiedbb
left a comment
jeremiedbb
left a comment
There was a problem hiding this comment.
LGTM. I like that now ipython can auto complete sklearn submodules.
doc/whats_new/v1.6.rst
Outdated
| - |Enhancement| Scikit-learn classes and functions can be imported from the root | ||
| `sklearn` module. :pr:`29793` by `Thomas Fan`_. |
There was a problem hiding this comment.
I find the formulation a bit misleading. One can think that he can now do from sklearn import LogisticRegression.
|
For me a key was to think about this PR as enabling a new/different style of Python, and not so much "lazy loading to make imports faster". I'm undecided if I like adding the "new style" or not. Not so much because I like/dislike the technical implementation but more because it adds a different way of doing the same thing. And on top of that I don't experience the problem that this solves. |
When working in interactive IPython sessions, it often takes as many import statements as useful code lines to reproduce a toy experiment interactively to prove a point during an educational live coding session or craft a minimal reproducer. |
|
Actually I like the point raised by @ogrisel. This is indeed something that I experienced. |
adrinjalali
left a comment
adrinjalali
left a comment
There was a problem hiding this comment.
Other than the changelog, LGTM.
Co-authored-by: Adrin Jalali <[email protected]>
doc/whats_new/v1.6.rst
Outdated
| More details in :ref:`estimator_tags`. | ||
| :pr:`22606` by `Thomas Fan`_ and :pr:`29677` by `Adrin Jalali`_. | ||
|
|
||
| - |Enhancement| Scikit-learn classes and functions can be used while only having a |
There was a problem hiding this comment.
You will need to add a fragment in doc/whats_new/upcoming_changes/many-modules/29793.enhancement.rst and have the following entry:
- Scikit-learn classes and functions can be used while only having a
`import sklearn` import line. For example, `import sklearn; sklearn.svm.SVC()` now works.
By :user:`Thomas Fan <thomasjpfan>`
6 participants
Reference Issues/PRs
Simpler version of #26440
What does this implement/fix? Explain your changes.
This PR implements lazy loading at the top level. With this PR, it enables users to write code like this: