-
Notifications
You must be signed in to change notification settings - Fork 1
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
[pdp] Map feature columns to human-friendly names #43
Conversation
shamelessly stolen from PR datakind/student-success-intervention#120
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.
I think we may want to update the unit test for inference as well to reflect the new output format. Once we do that, I think we're good to merge.
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.
Having human friendly names in our output feature file is a great idea. Definitely onboard with this. I'm also open to .toml file, I don't necessarily have a preference either way.
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.
Should we also update the unit test as well under tests/modeling/test_inference.py
?
Other than that, this looks good to me and similar to the changes in the private repo.
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.
Yeah, good call. I ported those tests over as-is from the private repo; will modernize and extend here.
changes
tomli
as optional project dependency (for PY3.10 only)context
PDP's internal feature column naming is a bit... technical. Advisors need more friendly naming to help them interpret model outputs correctly.
questions