-
Notifications
You must be signed in to change notification settings - Fork 0
Use level names for categories #25
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
Conversation
| :num_classes => (task == "Regression") ? -1 : length(y_classes), | ||
| :y_stat_given_feat_level => y_stat_given_feat_level, | ||
| :encoded_features => encoded_features, | ||
| :y_classes => (task == "Regression") ? nothing : y_classes, |
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.
This is fine but I'm curious, is there a reason to use a dictionary for the cache? More standard would be to use a named tuple, or if this needs to be mutable, a mutable struct.
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.
There isn't indeed and I plan to replace them all with named-tuples as in #5. Such action would be deterministic so I was waiting to merge the encoding types PR (and maybe this one) to avoid conflicts.
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.
Appreciate the non-trivial work needed here and the attention to detail in the tests. Many thanks 🙏🏾
Co-authored-by: Anthony Blaom, PhD <[email protected]>
Co-authored-by: Anthony Blaom, PhD <[email protected]>
Co-authored-by: Anthony Blaom, PhD <[email protected]>
Co-authored-by: Anthony Blaom, PhD <[email protected]>
Closes #10 it was tricky to implement because for some methods, the new columns added are not directly related to the levels of the column (eg, target encoding where it's related to the classes)