Remove pandas from gsaid api #3393
Conversation
|
cg/meta/upload/gisaid/gisaid.py
Outdated
| ) | ||
| reports: list[dict] = [report.model_dump() for report in sars_cov_complementary_reports] | ||
| write_csv_from_dict( | ||
| content=reports, fieldnames=HEADERS, file_path=Path(complementary_report.full_path) |
There was a problem hiding this comment.
What headers are required here and does it does it have to be in Swedish? @Clinical-Genomics/prodbioinfo
There was a problem hiding this comment.
I would keep the headers as is, this format was not defined by us iirc
|
@karlnyr This one is quite hard to test. Do you have any feedback on a proper way to test this? |
|
@Clinical-Genomics/prodbioinfo How does an invalid gisaid log entry look like? I.e. I cannot find one in stage. |
When already uploaded, 👍 |
|
Hard to test this more since one needs to make an actual upload to GISAID. Would be great with a review @Clinical-Genomics/sysdev @Clinical-Genomics/prodbioinfo ans suggestions how to further test this are very welcome. |
|
Would be great with a review @Clinical-Genomics/sysdev @Clinical-Genomics/prodbioinfo ans suggestions how to further test this are very welcome. |
|
| return gisaid_accession_count == sample_number_count | ||
|
|
||
| def upload_to_gisaid(self, case_id: str) -> None: | ||
| """Uploading results to GISAID and saving the accession numbers in complementary file.""" |
There was a problem hiding this comment.
| """Uploading results to GISAID and saving the accession numbers in complementary file.""" | |
| """Upload results to GISAID and save the accession numbers in complementary file.""" |
| sample_number: str = Field(str, alias="provnummer") | ||
| selection_criteria: str = Field(str, alias="urvalskriterium") |
There was a problem hiding this comment.
I think the str in the default position might cause a bug
| complementary_report_content_raw: list[dict] = self.get_complementary_report_content_raw( | ||
| case_id | ||
| ) | ||
| sars_cov_complementary_reports: list[GisaidComplementaryReport] = ( | ||
| self.parse_and_get_sars_cov_complementary_reports(complementary_report_content_raw) | ||
| ) |
There was a problem hiding this comment.
This same logic is done in the upload_to_gisaid function. Wouldn't it be better if this function got as a parameter the sars_cov_complementary_reports instead of calculating it again?
| LOG.info("GISAID log exists in case bundle in Housekeeper") | ||
| def create_and_include_gisaid_log_file_to_hk(self, case_id: str) -> None: | ||
| """Create and include GISAID log file to a Housekeeper.""" | ||
| if _ := self.housekeeper_api.get_files( |
There was a problem hiding this comment.
I don't think this walrus operator is needed here if the value is not going to be used
| if _ := self.housekeeper_api.get_files( | |
| if self.housekeeper_api.get_files( |
| def test_validate_gisaid_complementary_reports( | ||
| gisaid_api: GisaidAPI, gisaid_complementary_report_raw: dict[str, str] | ||
| ): | ||
| # GIVEN a dict |
There was a problem hiding this comment.
| # GIVEN a dict | |
| # GIVEN a valid GISAID complementary report as a dict |
| def test_add_gisaid_accession_to_reports( | ||
| gisaid_complementary_reports: list[GisaidComplementaryReport], gisaid_api: GisaidAPI | ||
| ): | ||
| """Test adding gisaid accession to the reports.""" |
There was a problem hiding this comment.
Should we stick to using capitals for GISAID?
| """Test adding gisaid accession to the reports.""" | |
| """Test adding GISAID accession to the reports.""" |
Description
Added
Changed
Fixed
How to prepare for test
uspaxaHow to test
Expected test outcome
Review
Thanks for filling in who performed the code review and the test!
This version is a
Implementation Plan