Data Labeler Compiler Diff #336
Conversation
grant-eden
requested review from
AnhTruong,
ChrisWallace2020,
JGSweets and
lettergram
as code owners
| 'c': 0.84 | ||
| } | ||
| }, | ||
| 'data_label': ['a', 'b'] |
There was a problem hiding this comment.
why is this data_label different than the one in statistics? Also, in profile do we have data_label at both levels? Should this instead pop it from the statistics level and reinsert it at the top level?
There was a problem hiding this comment.
good catch, diff in the datalabeler profile is not supposed to return data label
| @@ -233,6 +233,7 @@ def profile(self): | |||
| Property for profile. Returns the profile of the column. | |||
| """ | |||
| profile = { | |||
| "data_label": self.data_label, | |||
There was a problem hiding this comment.
@JGSweets @AnhTruong I've made a small refactor to put the data_label in the profile instead of having it extracted from the compiler level. Was there a specific reason the data_label was left out of the profile? Is this refactor inappropriate?
There was a problem hiding this comment.
I think this is appropriate.
The update above: profile["data_label"] = col_profile.pop("data_label") completes the refactor.
| # Test disabling both datalabeler profiles for compiler diff | ||
| compiler2 = col_pro_compilers.ColumnDataLabelerCompiler(data, options) | ||
| expected_diff = {} | ||
| self.assertDictEqual(expected_diff, compiler1.diff(compiler2)) |
There was a problem hiding this comment.
can we add a test with one profile is empty, then the result should be the other profile?
There was a problem hiding this comment.
The result wont be the other profile, it would be empty, since you cant have a difference unless you have 2 profiles to compare
…o datalabelercompilerdiff
…data-profiler into datalabelercompilerdiff
* implementing ability for null option * Woah. NICE. (#336) * revised unalikeability functionality * added test cases for revised unalikeability functionality * update to 0.6.1 * added ability for null values to be set by user choice * fixed test cases for test_profiler_options.py and test_profiler_class_options.py relating to null_values * fixed functionality for null_values * fixed functionality and documentation for null_values; added test case to cover more types * fixed syntax for null_values * fixed syntax for profiler_options.py * fixed syntax for profiler_options.py * fixed syntax for test cases * fixed syntax for test cases * fixed syntax for test cases * fixed syntax * fixed syntax Co-authored-by: Grant <56846128+gme5078@users.noreply.github.com>
* implementing ability for null option * Woah. NICE. (capitalone#336) * revised unalikeability functionality * added test cases for revised unalikeability functionality * update to 0.6.1 * added ability for null values to be set by user choice * fixed test cases for test_profiler_options.py and test_profiler_class_options.py relating to null_values * fixed functionality for null_values * fixed functionality and documentation for null_values; added test case to cover more types * fixed syntax for null_values * fixed syntax for profiler_options.py * fixed syntax for profiler_options.py * fixed syntax for test cases * fixed syntax for test cases * fixed syntax for test cases * fixed syntax * fixed syntax Co-authored-by: Grant <56846128+gme5078@users.noreply.github.com>
Pretty straightforward.
Might be strange that we have a for loop for one profile, but in the future that may change and it is consistent with the profile property.
Took a page out of Andrew's book for mocking.