Skip to content

Conversation

@Tuchentia
Copy link

No description provided.

Copy link
Collaborator

@LastShekel LastShekel left a comment

Choose a reason for hiding this comment

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

Hello!
Thank you for your interest in our project. We glad that you found the RFE hybrid method which is not implemented yet.
Here I left some comments on your code and, kindly ask you to write tests to it.
Thank you)

for f in features[:int(len(Xij) / 2)]:
Xk[-1].append(Xij[f])

svm = SVC(kernel='linear')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here you start to initialise some models, but you should set them as parameter (list of models, for example) and add it as a parameter into the init function.

from sklearn.ensemble import GradientBoostingClassifier


def hybrid_rfe(X_input, y_input, m=None, weighted=True, k=5, feature_names=None):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since you are creating a hybrid RFE you should create it as class

features_best = features

X_result = []
for Xi in X_input:
Copy link
Collaborator

Choose a reason for hiding this comment

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

this should be done via list comprehension (https://www.w3schools.com/python/python_lists_comprehension.asp)

svm.fit(Xk, yk)
rf.fit(Xk, yk)
gbm.fit(Xk, yk)
score = (svm.score(Xk, yk) + rf.score(Xk, yk) + gbm.score(Xk, yk)) / 3
Copy link
Collaborator

Choose a reason for hiding this comment

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

It is better to use this mean function as parameter

@Tuchentia Tuchentia requested a review from LastShekel May 20, 2021 11:04
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

Successfully merging this pull request may close these issues.

2 participants