fix: Issue422 replace print with logging #543
Conversation
|
/gcbrun |
|
@nehanene15 Do let me know if you also want me to change print statements in test cases |
data_validation/data_validation.py
Outdated
| print("-- ** Logging Target DF ** --") | ||
| print(target_df.dtypes) | ||
| print(target_df) | ||
| logging.info("-- ** Logging Source DF ** --") |
There was a problem hiding this comment.
these seem like error logs
| @@ -36,11 +37,11 @@ def print_formatted_(self, result_df): | |||
| :param result_df | |||
| """ | |||
| if self.format == "text": | |||
| print(result_df.to_string(index=False)) | |||
| logging.info(result_df.to_string(index=False)) | |||
There was a problem hiding this comment.
Does the logging change the nature of this printing (newlines might either split into new logs or be less exportable)
I think printing might be more logical here, but we need to maintain the ability to export into a file
There was a problem hiding this comment.
I don't think logging would split into new logs if it has \n in it.
https://stackoverflow.com/questions/22934616/multi-line-logging-in-python.
Do let me know if this makes sense to you
There was a problem hiding this comment.
It something I've seen in stackdriver specifically.
In any case, I'm more worried that you are changing the actual output here since this option is requesting a specific type to be output. CSV and json data would no longer be valid, so anyone piping these out to files would start seeing differences.
There was a problem hiding this comment.
That makes sense! Will update it
Close #419