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

[INF] Merge to BorutaPy (sklearn-contrib) #12

Closed
2 tasks
ThomasBury opened this issue Jul 1, 2020 · 3 comments
Closed
2 tasks

[INF] Merge to BorutaPy (sklearn-contrib) #12

ThomasBury opened this issue Jul 1, 2020 · 3 comments

Comments

@ThomasBury
Copy link

Description

Merge those to BorutaPy
I did a similar PR, with almost all the same modifications.

Reasoning

It makes sense to merge to the parent BorutaPy package to avoid duplicates
Existing PR can be then replaced or merge with the present package (FYI scikit-learn-contrib/boruta_py#77).

It'll be part of sklearn-contrib, which is actually a nice way to have visibility and a significant impact on the ML community.

Implementation

Overview of possible implementations

Tasks

  • Reach out the BorutaPy the maintainers, discuss the merging
  • Task 2
  • Task 3
@ThomasBury ThomasBury changed the title [INF] [INF] Merge to BorutaPy (sklearn-contrib) Jul 1, 2020
@Ekeany
Copy link
Owner

Ekeany commented Jul 1, 2020

Hi Thomas,

I have no issues with the proposed merge makes sense to unify all the methods into a single package.

@ThomasBury
Copy link
Author

Hi Ekeany,

I opened an issue, see here, to discuss the changes/merge. Feel free to be part of the discussion (or not).

There are great things in your package, I think I proposed +- the same in my PR but not necessarily as well written than your code. You can still have a look to the PR. Maybe there are some parts that you can re-use if the merge with Boruta-Py is not granted. In that case, I'll re-read carefully your code and if I have any proposition, I'll make a PR.

@ThomasBury
Copy link
Author

Hi Ekeany,

I'll close it since the Boruta owner doesn't seem in favour of a merge. Maybe, he'll reconsider at some point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants