Skip to content

MNT Make modules private in feature_extraction #15321

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Oct 25, 2019
Merged

MNT Make modules private in feature_extraction #15321

merged 7 commits into from
Oct 25, 2019

Conversation

thomasjpfan
Copy link
Member

@thomasjpfan thomasjpfan commented Oct 21, 2019 •
edited
Loading

Reference Issues/PRs

Partially addresses #9250

CC @NicolasHug @adrinjalali

@NicolasHug NicolasHug mentioned this pull request Oct 21, 2019
Closed
@thomasjpfan thomasjpfan changed the title [MRG] MNT Deprecated paths in feature_extraction [MRG] MNT Make modules private in feature_extraction Oct 21, 2019
NicolasHug
NicolasHug reviewed Oct 21, 2019
View reviewed changes
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.

glemaitre
glemaitre reviewed Oct 23, 2019
View reviewed changes
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.

@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 !

@cmarmo cmarmo mentioned this pull request Jul 18, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NicolasHug NicolasHug NicolasHug approved these changes

@glemaitre glemaitre glemaitre approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.22
Development

Successfully merging this pull request may close these issues.
4 participants
@thomasjpfan @adrinjalali @NicolasHug @glemaitre