Encode categorical column #770
Conversation
kshitijavis
requested review from
JGSweets,
ksneab7,
taylorfturner,
micdavis and
tyfarnan
as code owners
kshitijavis
force-pushed
the
encode_categorical_column
branch
from
5cfd109 to
9989370
Compare
| exepcted = json.dumps( | ||
| { | ||
| "name": profile.name, | ||
| "col_index": profile.col_index, | ||
| "sample_size": profile.sample_size, | ||
| "metadata": profile.metadata, | ||
| "times": profile.times, | ||
| "thread_safe": profile.thread_safe, | ||
| "_categories": profile._categories, | ||
| "_CategoricalColumn__calculations": {}, | ||
| "_top_k_categories": profile._top_k_categories, | ||
| } | ||
| ) |
There was a problem hiding this comment.
I would recommend making this more test-driven development.. instead of assigning expected values from the profile object itself.
There was a problem hiding this comment.
Yeah instead of using the profile. these values should be literals. This is to prevent changes from going under the radar and forcing the test to have specific values, rather than change with your profile object.
| profile.update(df_categorical) | ||
|
|
||
| serialized = json.dumps(profile, cls=ProfileEncoder) | ||
| exepcted = json.dumps( |
| } | ||
| ) | ||
|
|
||
| self.assertEqual(json.loads(serialized), json.loads(expected)) |
There was a problem hiding this comment.
if we are trying to test the functionality of encoding and decoding (profile -> json -> profile) then I think that the assertEqual() should be comparing the decoded data (json -> profile) to another profile object that represents the object we expect as a result.
| exepcted = json.dumps( | ||
| { | ||
| "name": profile.name, | ||
| "col_index": profile.col_index, | ||
| "sample_size": profile.sample_size, | ||
| "metadata": profile.metadata, | ||
| "times": profile.times, | ||
| "thread_safe": profile.thread_safe, | ||
| "_categories": profile._categories, | ||
| "_CategoricalColumn__calculations": {}, | ||
| "_top_k_categories": profile._top_k_categories, | ||
| } | ||
| ) |
taylorfturner
added
Medium Priority
Head branch was pushed to by a user without write access
Head branch was pushed to by a user without write access
No description provided.