Align test/id sdtypes to match SDV
#881
Conversation
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #881 +/- ##
==========================================
Coverage 100.00% 100.00%
==========================================
Files 18 18
Lines 1844 2215 +371
==========================================
+ Hits 1844 2215 +371 ☔ View full report in Codecov by Sentry. |
| @@ -1,4 +1,4 @@ | |||
| """Dataset Generators for Text transformers.""" | |||
| """Dataset Generators for 'text' transformers.""" | |||
There was a problem hiding this comment.
Should we just delete this file now since we have the id one testing the exact same thing?
There was a problem hiding this comment.
I left it so we can test the text input still works as sdtype.
rdt/performance/datasets/id.py
Outdated
|
|
||
| @staticmethod | ||
| def get_performance_thresholds(): | ||
| """Return the expected threseholds.""" |
There was a problem hiding this comment.
| """Return the expected threseholds.""" | |
| """Return the expected thresholds.""" |
May want to Ctrl+F the library for this typo
| "Importing 'IDGenerator' or 'RegexGenerator' for ID columns from 'rdt.transformers.text' " | ||
| "is deprecated. Please use 'rdt.transformers.id' instead.", |
There was a problem hiding this comment.
This warning mentions importing from transformers.id instead of transformers.text but should it also mention switching the sdtype from text to id?
There was a problem hiding this comment.
Not sure, cc: @npatki
There was a problem hiding this comment.
Sorry I revisited the issue and we would like to raise a warning if the sdtype is set as text.
There was a problem hiding this comment.
Addressed here 2a8d014
rdt/transformers/utils.py
Outdated
| """Custom dictionary to raise a deprecation warning.""" | ||
|
|
||
| def get(self, key): | ||
| """Retrun the value for key if key is in the dictionary, else default. |
There was a problem hiding this comment.
| """Retrun the value for key if key is in the dictionary, else default. | |
| """Return the value for key if key is in the dictionary, else default. |
| 'text': RegexGenerator(), | ||
| 'pii': AnonymizedFaker(), | ||
| } | ||
| DEFAULT_TRANSFORMERS = WarnDict( |
There was a problem hiding this comment.
Does this cause the warning to be raised more than once in one workflow? The cases I'm wondering are
- If the user has multiple columns set to an sdtype of text
- If the overall process for the HyperTransformer accesses this dict more than once
We probably want to make sure the user doesn't get spammed with this warning
pvk-developer
deleted the
issue-880-align-text-id-sdtypes-to-sdv-library
branch
Resolves #880
CU-86b2378fg