Repository navigation
Add pct_null_rows to HighlyNullDataCheck - #2211
Conversation
| "level": "warning",\ | ||
| "code": "HIGHLY_NULL",\ | ||
| "details": {"column": "lots_of_null"}}],\ | ||
| "details": {"column": "lots_of_null", "pct_null_rows": 0.8}}],\ |
There was a problem hiding this comment.
Open to suggestion: keep as 0.8 or display as percentage 80%?
There was a problem hiding this comment.
Maybe display as percentage since the field name has the word percent in it? I don't feel super strongly about it though
There was a problem hiding this comment.
I think 0.8 is better for now. I believe use [0, 1] notation for percent baseline, class imbalance ratios in params and other fields. I think its nice having our codebase be consistent with how we represent percentages.
Codecov Report
@@ Coverage Diff @@
## main #2211 +/- ##
=======================================
Coverage 100.0% 100.0%
=======================================
Files 287 287
Lines 24451 24451
=======================================
Hits 24433 24433
Misses 18 18
Continue to review full report at Codecov.
|
freddyaboulton
left a comment
freddyaboulton
left a comment
There was a problem hiding this comment.
Looks good @jeremyliweishih !
| "level": "warning",\ | ||
| "code": "HIGHLY_NULL",\ | ||
| "details": {"column": "lots_of_null"}}],\ | ||
| "details": {"column": "lots_of_null", "pct_null_rows": 0.8}}],\ |
There was a problem hiding this comment.
Maybe display as percentage since the field name has the word percent in it? I don't feel super strongly about it though
angela97lin
left a comment
angela97lin
left a comment
There was a problem hiding this comment.
LGTM! Don't feel too strongly about your suggestion, looks fine as is.
| "level": "warning",\ | ||
| "code": "HIGHLY_NULL",\ | ||
| "details": {"column": "lots_of_null"}}],\ | ||
| "details": {"column": "lots_of_null", "pct_null_rows": 0.8}}],\ |
There was a problem hiding this comment.
I think 0.8 is better for now. I believe use [0, 1] notation for percent baseline, class imbalance ratios in params and other fields. I think its nice having our codebase be consistent with how we represent percentages.
| data_check_name=highly_null_data_check_name, | ||
| message_code=DataCheckMessageCode.HIGHLY_NULL, | ||
| details={"column": "lots_of_null"}).to_dict(), | ||
| details={"column": "lots_of_null", "pct_null_rows": 0.8}).to_dict(), |
There was a problem hiding this comment.
Ah great, I was looking for a test case where this wasn't 1.0 :)
4 participants
Fixes #2201.