[MRG] MNT removed deprecated files generation #17132
Conversation
|
you've been sitting on this, haven't you :D It'd be nice to merge this a bit after release I'd say. |
.gitignore
Outdated
| # TODO: Remove in 0.24 | ||
| # All of these files should have a match in _build_utils/deprecated_modules.py | ||
| # All of these files were deprecated and automatically generated in 0.22, and | ||
| # removed in 0.24. We keep them in gitignore otherwise users would need to |
There was a problem hiding this comment.
| # removed in 0.24. We keep them in gitignore otherwise users would need to | |
| # removed in 0.24. We keep them in gitignore otherwise devs would need to |
|
@glemaitre @adrinjalali @thomasjpfan @rth @amueller @jnothman easy one? |
|
conflicts |
…move_deprecated_modules
|
thanks should be good now |
|
thanks for the review!
Yeah, we started late in 0.21 so it's basically only been 6 months, i.e. one release |
There was a problem hiding this comment.
Otherwise LGTM, thanks @NicolasHug . I'm quite proud of us having done this!
.gitignore
Outdated
| # All of these files were deprecated and automatically generated in 0.22, and | ||
| # removed in 0.24. We keep them in gitignore otherwise devs would need to | ||
| # manually delete them |
There was a problem hiding this comment.
git clean -xfd does that though. I'd rather remove them from here and have us run the command once?
There was a problem hiding this comment.
Ok, I'm fine with either.
We should probably recommend git clean -xi since this will remove all untracked files. We can do that in a sticky issue I guess?
Pinging @glemaitre @rth @thomasjpfan @ogrisel @jnothman , any objection?
There was a problem hiding this comment.
I would prefer to remove them from .gitignore.
Although, I do not know where to put the git clean information. We would most likely need to help contributors with this when they sync with master and then use git add . which would add these ignored files back.
There was a problem hiding this comment.
thoughts on a sticky issue as suggested?
There was a problem hiding this comment.
Not sure what they are, but I like the sound of it
There was a problem hiding this comment.
We can pin issues to the top as done in https://github.com/Microsoft/LightGBM/issues?q=is%3Aissue+is%3Aopen+sort%3Aupdated-desc
There was a problem hiding this comment.
We didn't get a chance to discuss this during the meeting so I'll merge tomorrow unless someone objects
…move_deprecated_modules
| @@ -474,8 +474,6 @@ def test_partial_dependence_X_list(estimator): | |||
| partial_dependence(estimator, list(X), [0]) | |||
|
|
|||
|
|
|||
| # TODO: Remove in 0.24 when DummyClassifier's `strategy` default updates | |||
There was a problem hiding this comment.
these 2 aren't related to module deprecation but I think they're the only ones remaining so I'll squeeze them here instead of opening another PR
…move_deprecated_modules
…move_deprecated_modules
|
LGTM Thanks @NicolasHug |
|
Thanks @glemaitre Created and pinned #17341 |
No description provided.