-
Notifications
You must be signed in to change notification settings - Fork 393
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
Adding key
ctor field in all RawFeatureFilter results
#348
Conversation
Codecov Report
@@ Coverage Diff @@
## master #348 +/- ##
========================================
Coverage 86.65% 86.65%
========================================
Files 335 335
Lines 10763 10763
Branches 344 562 +218
========================================
Hits 9327 9327
Misses 1436 1436
Continue to review full report at Codecov.
|
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.
-
Please add
key
comparison in the comparison functions ofRawFeatureFilterMetrics
andExclusionReasons
in fileRawFeatureFilterResultsComparison.scala
. -
Please update the tests in
RawFeatureFilterTest.scala
to include key.
key
ctor field in all RawFeatureFilter resultskey
ctor field in all RawFeatureFilter results
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.
LGTM.
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.
Lgtm, see minor comment
core/src/main/scala/com/salesforce/op/filters/RawFeatureFilterResults.scala
Outdated
Show resolved
Hide resolved
core/src/main/scala/com/salesforce/op/filters/RawFeatureFilterResults.scala
Show resolved
Hide resolved
Thanks for the contribution! It looks like @mweilsalesforce is an internal user so signing the CLA is not required. However, we need to confirm this. |
Related issues
FeatureDistribution
contains the fieldkey
which is the map key associated with distribution (when the feature is a map). But why not having the same field inRawFeatureFilterMetrics
andExclusionReasons
?Describe the proposed solution
Adding the same field in
RawFeatureFilterMetrics
andExclusionReasons
Describe alternatives you've considered
Additional context