Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Functionality to let LightGBM effectively handle categorical features #1585
Functionality to let LightGBM effectively handle categorical features #1585
Changes from 20 commits
ec8fe6d
5ee855d
149b2c7
b02e8b1
948be36
3c2fee2
c3d642f
5679eeb
ef7fcf8
5a5a09f
4c5b140
f6b25fc
e7cde27
d8aa69f
5be4f4c
dc9ceeb
713a850
165d1bc
d02d3a0
95bf521
9df90ae
36e56de
e85bad2
9ba3190
5f2535b
ae1d4df
7cb8c72
20073fe
6eb4ed4
5dc1341
fc41cd8
0836ff2
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could we also allow single strings?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could make this a private method (like
_check_categorical_covariates
) of this new base class mentioned in the earlier comment, so we can later reuse it for the other models support categorical covariates.This method would always be called by all models inheriting from the new "base" class
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in the new "base" class you can override
_fit_model()
and avoid have the same logic in two places.something like below:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A mapping for getting the correct parameter name per model could allow to dynamically provide the categorical features.
i.e. "cat_features" for CatBoost, "categorical_features" for LightGBM
self.categorical_fit_param_name = "categorical_features"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in the new "base" class we could drop this check and
SupportsCategoricalCovariates
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can remove the
else
and unindentThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is the
if cat
required?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could be removed if we go with the new "base" class