manuel-calzolari / sklearn-genetic

Genetic feature selection module for scikit-learn
https://sklearn-genetic.readthedocs.io
GNU Lesser General Public License v3.0
324 stars 77 forks source link

for n_components in PLSRegression #36

Closed jckkvs closed 9 months ago

jckkvs commented 1 year ago

When using PLSregression, it is possible that the number of variables selected by the GA exceeds the set n_components. This will result in a ValueError depending on the version of sklearn.

I reset n_components to avoid that ValueError

manuel-calzolari commented 1 year ago

Hi @jckkvs, thank you for your contribution!

I would prefer not to silently overwrite the value of "n_components", maybe we could just add the "min_features" parameter?

jckkvs commented 1 year ago

@manuel-calzolari

Thank you for your reply. I believe there are two ways to address the n_components issue I raised. One is by overwriting the n_components value for PLSRegression, and the other is by setting the min_features.

Initially, I was leaning towards the first approach, but now I have submitted a pull request with the second code. I apologize for not updating the comments accordingly.

https://github.com/manuel-calzolari/sklearn-genetic/pull/36/files

manuel-calzolari commented 1 year ago

@jckkvs Would it be possible to update the branch linked to this PR by resolving conflicts and eliminating the first approach (leaving only the addition of the min_features parameter)? Otherwise, I can simply close this PR and do a commit myself to add min_features.

manuel-calzolari commented 1 year ago

@jckkvs Can we keep only the addition of min_features for this Pull Request?

jckkvs commented 1 year ago

@manuel-calzolari Certainly, let's proceed with min_features only.

jckkvs commented 1 year ago

Should I make the code changes, or will you?

manuel-calzolari commented 1 year ago

Should I make the code changes, or will you?

@jckkvs If the adjustments could be made directly within the branch associated with this Pull Request, it would streamline the process on my end. Otherwise, I would need to create a new branch, selectively cherry-pick the desired commits, and then squash them.

jckkvs commented 1 year ago

@manuel-calzolari I removed code "auto_n_components"

jckkvs commented 1 year ago

@manuel-calzolari I updated.