[MRG] ENH Allows private methods to be imported#15419
[MRG] ENH Allows private methods to be imported#15419thomasjpfan wants to merge 11 commits intoscikit-learn:masterfrom
Conversation
|
ping when ready please! |
|
@NicolasHug Ready now. Edit not true |
|
Test by importing imblearn's modules?
|
|
Tested this PR with |
|
Most checks are failing @thomasjpfan |
There was a problem hiding this comment.
A few comments.
I don't like that you are updating the new module's __all__.
Can't we just set the attributes of the generated module?
I think we should also consider the backport proposed by @rth and compare.
| # THIS FILE WAS AUTOMATICALLY GENERATED BY deprecated_modules.py | ||
| from . import {new_module_name} | ||
|
|
||
| if {new_module_name}.__file__.endswith(".py"): |
There was a problem hiding this comment.
It turns out that only _ball_tree.pyx and _kd_tree.pyx defines __all__. Files such as _tree.pyx that does not define __all__ the from from sklearn.tree.tree import * actually imports everything, such as Tree.
In short, even without this PR, importing from the compiled modules works, such as from sklearn.tree.tree import Tree.
There was a problem hiding this comment.
what about _ball_tree.pyx and _kd_tree.pyx then?
There was a problem hiding this comment.
All the other functions inside those files where cdef so they were not able to be imported in the first place.
There was a problem hiding this comment.
Well, cython files could. But I guess we don't care.
| if not hasattr({new_module_name}, '__all__'): | ||
| {new_module_name}.__all__ = [] |
There was a problem hiding this comment.
maybe avoid the condition with something like new_module.__all__ = getattr(new_module, '__all__', [])?
We can't use pep 562 because its >=3.7, so the only option is adding to |
This reverts commit 431b9bb.
|
Ah okay, we can backport it. |
|
Backport alternative at #15431 |
|
#15431 was merged, so we can probably close this? Thanks @thomasjpfan ! |
Reference Issues/PRs
Resolves #15415
What does this implement/fix? Explain your changes.
Adds methods and classes to
__all__dynamically and thenimport *Any other comments?