Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Now json_summary custom log filename includes module called #278

Merged
merged 5 commits into from
Jul 16, 2024

Conversation

Shettland
Copy link
Member

Appart from a small fix in read_lab_metadata (Now when an invalid date is met is doesn't crash, just continues), now the output json file with the custom log from json_summary.py includes the name of the module called (e.g. if its generated from a read-lab-metadata call, the output name will be DATE_read_lab_metadata_log_summary.json

@@ -307,7 +308,7 @@ def read_metadata_file(self):
log_text = f"Invalid date format in sample {str(key)}"
self.logsum.add_error(sample_id, log_text)
stderr.print(f"[red]{log_text}")
row[key] = None
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the validation process checks for date format? If so this is ok as we'll get that information in the validation process, but if not, maybe we need to keep the "None" so we don't have a validated date field it it is not a date?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does not check date format, but the functionality of this "continue" is to not include this field in the final json. I don't think it makes much sense to include it as "None" instead of just removing it

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, so then if it is required it won't validate. In any case make sure this information is not lost and is kept in the json-log so when we ask the hospital for modifications the problem is clear

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm to save the exact field in json logs i'd need a small change, I'll update it

@Shettland Shettland merged commit c4af29f into BU-ISCIII:develop Jul 16, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants