Add verbosity to drop_unknown_references
#1854
Conversation
1 similar comment
| 'Success! All foreign keys have referential integrity.\n' | ||
| 'No rows were dropped.' | ||
| ) | ||
| assert captured.out.strip() == expected_message |
There was a problem hiding this comment.
I would rather we not have a special case message for this case. In the spec, we said we should print out for every table all the time.
Even if nothing was dropped, I think it's useful reassurance to the user that we checked all tables. Plus, it simplifies our logic.
There was a problem hiding this comment.
Alright, done in 7a80b0a
|
@R-Palazzo I also think that the first case you have added a line |
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 #1854 +/- ##
=======================================
Coverage 97.32% 97.33%
=======================================
Files 51 51
Lines 4823 4834 +11
=======================================
+ Hits 4694 4705 +11
Misses 129 129 ☔ View full report in Codecov by Sentry. |
sdv/utils/poc.py
Outdated
| metadata.validate() | ||
| try: | ||
| metadata.validate_data(data) | ||
| if drop_missing_values: | ||
| _validate_foreign_keys_not_null(metadata, data) | ||
|
|
||
| if verbose: | ||
| sys.stdout.write('\n'.join([success_message, summary_table.to_string(index=False)])) |
There was a problem hiding this comment.
Why sys.stdout instead of print (just curious).
Should we add one more \n, this is how it looks:
I think that this is a bit more digestable by the eye:
There was a problem hiding this comment.
Thanks, done in 6067e02. We have a lint error when using print but I could also add # noqa: T001 (in SDMetrics we use sys.stdout)
CU-86aznj0ht
Resolve #1845
@npatki I slightly changed the message printed compared to the issue let me know if it works: