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

Standardize Capitalization #243

Closed
mepland opened this issue Jan 9, 2023 · 3 comments
Closed

Standardize Capitalization #243

mepland opened this issue Jan 9, 2023 · 3 comments
Labels
Milestone

Comments

@mepland
Copy link
Collaborator

mepland commented Jan 9, 2023

We should standardize the capitalization in the plots, particularly on the axis labels where we have things like feature, leaf ids, samples count as well as GINI. Wouldn't Feature, Leaf IDs, Samples Count, and Gini be preferable?

@parrt
Copy link
Owner

parrt commented Jan 14, 2023

Agreed. Capitalized per English rules is probably a good idea, unless it's an acronym like MSE. is Gini an acronym? The other caveat is, it should be consistent with the decision tree library. If the library uses 'mse'to indicate the metric then we should use mse not MSE.

@mepland
Copy link
Collaborator Author

mepland commented Jan 14, 2023

Gini is a name, not an acronym. In this case the all uppercase style is coming from the ShadowSKDTree implementation:

    def criterion(self):
        return self.tree_model.criterion.upper()

We should remove the .upper() and take whatever criterion is provided by the model's library.

Sounds good, I'll make a PR to address this issue once #236 and #241 are merged!

@parrt parrt added the clean up label Jan 14, 2023
@parrt parrt modified the milestone: 2.1 Jan 14, 2023
@mepland
Copy link
Collaborator Author

mepland commented Jan 15, 2023

Addressed in #245!

@parrt parrt added this to the 2.1 milestone Jan 15, 2023
@parrt parrt closed this as completed Jan 15, 2023
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