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

MNT Make modules private in feature_extraction #15321

Merged

Conversation

thomasjpfan
Copy link
Member

@thomasjpfan thomasjpfan commented Oct 21, 2019

Reference Issues/PRs

Partially addresses #9250

CC @NicolasHug @adrinjalali

@thomasjpfan thomasjpfan changed the title [MRG] MNT Deprecated paths in feature_extraction [MRG] MNT Make modules private in feature_extraction Oct 21, 2019
Copy link
Member

@NicolasHug NicolasHug left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also deprecate cython files ;)

There are stuff in text that should be private I think, like the VectorizerMixin?

Same for image.extract_patches... Not sure what to do with these.

.gitignore Show resolved Hide resolved
sklearn/_build_utils/deprecated_modules.py Show resolved Hide resolved
Copy link
Member

@glemaitre glemaitre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LTGM apart of the comment of @NicolasHug which need to be addressed.

@glemaitre glemaitre added this to TO BE MERGED in Guillaume's pet Oct 23, 2019
@thomasjpfan
Copy link
Member Author

There are stuff in text that should be private I think, like the VectorizerMixin? Same for image.extract_patches... Not sure what to do with these.

Two options:

  1. Move to _text and _image and deprecate them.
  2. Move to _text and _image and remove them. (They are not documented)

@adrinjalali
Copy link
Member

I would move them to _* and deprecate the public ones (through text and image), and not include the undocumented ones in __init__.

@thomasjpfan
Copy link
Member Author

Updated this PR with a simplier approach. I didn't create another file and deprecated VectorizerMixin and extract_patches in the same file.

@thomasjpfan
Copy link
Member Author

CC @NicolasHug

@thomasjpfan thomasjpfan added this to the 0.22 milestone Oct 25, 2019
@NicolasHug NicolasHug changed the title [MRG] MNT Make modules private in feature_extraction MNT Make modules private in feature_extraction Oct 25, 2019
@NicolasHug NicolasHug merged commit bb3b3f9 into scikit-learn:master Oct 25, 2019
@NicolasHug
Copy link
Member

Thanks @thomasjpfan !

@glemaitre glemaitre moved this from TO BE MERGED to MERGED in Guillaume's pet Oct 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants