Issue 2921 and 2920 overlaping check - #2938
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2938 +/- ##
==========================================
+ Coverage 98.06% 98.08% +0.01%
==========================================
Files 75 76 +1
Lines 8589 8655 +66
==========================================
+ Hits 8423 8489 +66
Misses 166 166
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| ) | ||
|
|
||
|
|
||
| def _validate_data(real_data, synthetic_data, table_name, column_names): |
There was a problem hiding this comment.
maybe add a validation to check:
table_nameis a stringcolumn_namesis a list of strings
| if verbose: | ||
| print(f'Number of common combinations: {num_common} ({percent}%)') # noqa: T201 | ||
| if num_common == 0: | ||
| print(NO_OVERLAP_MESSAGE) # noqa: T201 |
There was a problem hiding this comment.
you can use sys.stdout.write instead of print
|
|
||
| MISSING_VALUE_PLACEHOLDER = '__sdv_missing_value__' | ||
|
|
||
| NO_OVERLAP_MESSAGE = ( |
There was a problem hiding this comment.
nit: since these messages are only used once, I would prefer to move them inside the function that uses them instead of creating a constant variable.
| real_values = real_data[table_name][column_names].copy() | ||
| synthetic_values = synthetic_data[table_name][column_names].copy() | ||
| for column_name in column_names: | ||
| real_values[column_name], synthetic_values[column_name] = _align_dtypes( |
There was a problem hiding this comment.
is this step necessary given that all the columns will be casted as 'object' in _get_combinations?
| # Assert | ||
| real_combinations = set(real_data[column_names].itertuples(index=False, name=None)) | ||
| assert isinstance(result, int) | ||
| assert 0 <= result <= len(real_combinations) |
There was a problem hiding this comment.
isn't the upper limit here the union of the unique combinations of real and synthetic?
| return {'table': real_table}, {'table': synthetic_table} | ||
|
|
||
|
|
||
| class TestGetCombinations: |
There was a problem hiding this comment.
If we are testing a function, then the test will be written as a function. If we are testing a class, then the test will be written as a class.
In this case, _get_combinations is a function and the test function definition should be something like test_{function_name}_{purpose_of_test}.
…al and synthetic data
Add utils.py with two functions that measure how much the synthetic data reuses values from the real data.
get_combination_overlap counts combinations of column values that appear in both datasets
get_pii_overlap does the same for the values of a single PII column.
Resolves #2921
Resolves #2920