-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
Make all non-canonical modules private? #9250
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
Comments
I would support this as it seems like it could clear up a lot of confusion. Do you believe there are any people directly trying to use the functions that would be made private? |
I am certain that there is code importing from the submodules, but
deprecation is fine. we might need to pay attention to whether or not base
class implementations get imported into the public modules
…On 30 Jun 2017 6:13 am, "Jacob Schreiber" ***@***.***> wrote:
I would support this as it seems like it could clear up a lot of
confusion. Do you believe there are any people directly trying to use the
functions that would be made private?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#9250 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAEz681uJKmev3Om4l4YDjvFQ9od_qqaks5sJAVxgaJpZM4OJW9F>
.
|
Since #14939 is merged, we seem to be deprecating them already for 0.22. This now seems like an easy one for some of the upcoming sprints. Although if we want it done before the end of october, we should do it ourselves maybe. |
Below is the list of the folders whose files need to be preceded by an underscore. The procedure is pretty straightforward:
Folders
(Note that this only addresses the folders. Files like |
ping @thomasjpfan @glemaitre @adrinjalali let's pick one each and review each-other? this should go relatively fast I'll pick |
I'm taking the |
I'll take |
pretty sure folks have not been using |
Squash merges will also mess that up.
|
I did move, as you see in this commit df7f54d but once you create a file with the same name, github gets confused. |
I'm working on |
Working on |
working on |
working on metrics and metrics/cluster |
I'll work on datasets |
Working on gaussian_process |
Working on feature_extraction |
Working on inspection |
Working on preprocessing |
Working on linear_models |
@thomasjpfan From the list, I only did not review the PR for the decomposition. I did not open a PR because you seem to have work on it. Could you open a PR? |
ping @NicolasHug @thomasjpfan what about sklearn.feature_selection? Is #15367 a blocker? |
Working on feature_selection |
Thanks I missed this one
Yes but it's tracked by another issue |
I had a ton of private datasets, such as names, text and geographical data. I used to use the following utils to download and manage these: from sklearn.datasets.base import (get_data_home, RemoteFileMetadata, _fetch_remote) What's the recommended approach now? |
you can import However, now you are aware that This said, be aware that we are going to define a developer API which will define some backward support for these types of utilities used in third-party. |
- Deprecate presort (scikit-learn/scikit-learn#14907) - Add Minimal Cost-Complexity Pruning to Decision Trees (scikit-learn/scikit-learn#12887) - Add bootstrap sample size limit to forest ensembles (scikit-learn/scikit-learn#14682) - Fix deprecated imports (scikit-learn/scikit-learn#9250)
- Deprecate presort (scikit-learn/scikit-learn#14907) - Add Minimal Cost-Complexity Pruning to Decision Trees (scikit-learn/scikit-learn#12887) - Add bootstrap sample size limit to forest ensembles (scikit-learn/scikit-learn#14682) - Fix deprecated imports (scikit-learn/scikit-learn#9250) Do not add ccp_alpha to SurvivalTree, because it relies node_impurity, which is not set for SurvivalTree.
- Deprecate presort (scikit-learn/scikit-learn#14907) - Add Minimal Cost-Complexity Pruning to Decision Trees (scikit-learn/scikit-learn#12887) - Add bootstrap sample size limit to forest ensembles (scikit-learn/scikit-learn#14682) - Fix deprecated imports (scikit-learn/scikit-learn#9250) Do not add ccp_alpha to SurvivalTree, because it relies node_impurity, which is not set for SurvivalTree.
- Deprecate presort (scikit-learn/scikit-learn#14907) - Add Minimal Cost-Complexity Pruning to Decision Trees (scikit-learn/scikit-learn#12887) - Add bootstrap sample size limit to forest ensembles (scikit-learn/scikit-learn#14682) - Fix deprecated imports (scikit-learn/scikit-learn#9250) Do not add ccp_alpha to SurvivalTree, because it relies node_impurity, which is not set for SurvivalTree.
This is maybe for 1.0. Recently we started marking files like
model_selection._split
private, so that everything has a single canonical import:For many (older?) models that's not the case, we have
etc.
I think it would be nice to make all the files that are not the canonical import (according to the API documentation) private (with deprecation obviously).
That might be a bit annoying for existing users that used long import paths, but it makes auto-complete much more helpful and the module structure much less confusing for newcomers.
For example
sklearn.linear_model.ridge
is a module, whilesklearn.linear_model.ridge_regression
is a function that implements ridge regression andsklearn.linear_model.Ridge
is a class that implements ridge regression. From the names this is totally unclear.The text was updated successfully, but these errors were encountered: