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-package] Renaming feature_name and categorical_feature attributes and setters #6169

Closed
Eyal8 opened this issue Oct 31, 2023 · 2 comments
Labels

Comments

@Eyal8
Copy link

Eyal8 commented Oct 31, 2023

I would like to suggest changing the attribute feature_name and categorical_feature to feature_names and categorical_features respectively, as they both represent lists, and their current naming is misleading.

Example of where to change:

feature_name: _LGBM_FeatureNameConfiguration = 'auto',
categorical_feature: _LGBM_CategoricalFeatureConfiguration = 'auto',

However, the use of these attributes is apparent in many additional places.
In addition I would suggest renaming their corresponding setter methods set_feature_name and set_categorical_feature in all the places, e.g.:

def set_feature_name(self, feature_name: _LGBM_FeatureNameConfiguration) -> "Dataset":

def set_categorical_feature(

@jameslamb jameslamb changed the title Renaming feature_name and categorical_feature attributes and setters [python-package] Renaming feature_name and categorical_feature attributes and setters Oct 31, 2023
@jameslamb
Copy link
Collaborator

Thanks for your interest in LightGBM.

I'm -1 on this suggestion. I agree that set_categorical_features() (plural) and similar would be clearer, but at this point the use of the singular form (categorical_feature, feature_name) is a significant part of the project's public API. I don't support breaking users' code in exchange for the slight improvement in clarity of the names.

I'll wait to close this for a few days to give @shiyu1994 @guolinke @jmoralez a chance to offer their opinions.

@jameslamb
Copy link
Collaborator

I'll wait to close this for a few days

turns out that was actually 8 months 😂

I'm going to close this. Thanks very much for taking the time to write this up @Eyal8 , we hope you'll come back to contribute again in the future.

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

No branches or pull requests

2 participants