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

[python] added get_split_value_histogram method #2041

Merged
merged 2 commits into from Mar 9, 2019
Merged

[python] added get_split_value_histogram method #2041

merged 2 commits into from Mar 9, 2019

Conversation

StrikerRUS
Copy link
Collaborator

@StrikerRUS StrikerRUS commented Mar 8, 2019

Closed #2013.

Calculate feature split histogram in XGBoost-style (https://github.com/dmlc/xgboost/blob/master/python-package/xgboost/core.py#L1694-L1737).

UPD: … and in standard (numpy) style.

@StrikerRUS
Copy link
Collaborator Author

Is docstring OK? I tried to make it informative and simple at the same time, but I'm still not satisfied 😕

@guolinke
Copy link
Collaborator

guolinke commented Mar 9, 2019

@StrikerRUS it looks good to me

@StrikerRUS
Copy link
Collaborator Author

OK, thanks! Can we merge then or second review is needed?

@guolinke
Copy link
Collaborator

guolinke commented Mar 9, 2019

@StrikerRUS I think it is ready for merge

@guolinke guolinke merged commit 8d6666e into master Mar 9, 2019
@StrikerRUS StrikerRUS deleted the hist branch March 9, 2019 16:53
@StrikerRUS
Copy link
Collaborator Author

StrikerRUS commented Mar 9, 2019

@guolinke I've found that it crashes with categorical feature (threshold has string type). What do you think we can do with it? Simply raise error for cat. feature or is there any smarter solution?

@guolinke
Copy link
Collaborator

@StrikerRUS
can we use a string type directly for the x-axis in histogram for categorical features?

@StrikerRUS
Copy link
Collaborator Author

numpy.histogram() crashes with string type. Also, how to sort cat. string values, let say ['0', '3', '2', '0||3']?

@guolinke
Copy link
Collaborator

@StrikerRUS yeah, the combination is a problem. Let us skip the categorical features for now.

@StrikerRUS
Copy link
Collaborator Author

@guolinke OK. Created #2045 for this.

@StrikerRUS StrikerRUS mentioned this pull request Apr 30, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Mar 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

get_split_value_histogram XGBoost equivalent in LightGBM
2 participants