Skip to content

Conversation

@remo-help
Copy link
Contributor

This PR adds a simple utility method to the Learner class, allowing it to return the feature names used at the estimator step as a 1-D array.
Learners with a selector potentially have a smaller feature name list than is stored in the feature_vectorizer. This method allows for easy access of the actual feature names of features used, even if the selector has removed specific features.

@remo-help remo-help linked an issue Jun 28, 2022 that may be closed by this pull request
@remo-help remo-help changed the title Added a utility method to the Learner class returning feature names [WIP] Added a utility method to the Learner class returning feature names Jun 28, 2022
@remo-help
Copy link
Contributor Author

hold off on review while I add tests

@codecov
Copy link

codecov bot commented Jun 28, 2022

Codecov Report

Base: 96.90% // Head: 96.90% // No change to project coverage 👍

Coverage data is based on head (bdf6107) compared to base (bdf6107).
Patch has no changes to coverable lines.

❗ Current head bdf6107 differs from pull request most recent head f3d5658. Consider uploading reports for the commit f3d5658 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #715   +/-   ##
=======================================
  Coverage   96.90%   96.90%           
=======================================
  Files          63       63           
  Lines        9271     9271           
=======================================
  Hits         8984     8984           
  Misses        287      287           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Collaborator

@desilinguist desilinguist left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty good but I have some suggestions - some mostly cosmetic but one to actually increase the test coverage check which is failing right now.

Copy link
Contributor Author

@remo-help remo-help left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

addressed PR comments

@remo-help remo-help requested a review from desilinguist July 29, 2022 18:58
@remo-help remo-help changed the title [WIP] Added a utility method to the Learner class returning feature names Added a utility method to the Learner class returning feature names Aug 1, 2022
Added a utility method to the Learner class, allowing it to return an
array of feature names after the selector step.
@desilinguist desilinguist force-pushed the add_feature_utility_method branch from c35ce14 to f3d5658 Compare September 12, 2022 19:29
@desilinguist desilinguist removed their request for review September 12, 2022 20:12
@desilinguist desilinguist dismissed their stale review September 13, 2022 18:03

All comments addressed.

@desilinguist desilinguist merged commit 7660981 into main Sep 13, 2022
@delete-merged-branch delete-merged-branch bot deleted the add_feature_utility_method branch September 13, 2022 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add class method to learner object for retrieving feature names

5 participants