Add exclusion criteria for ENA samples without collection date #144
Conversation
Added test for collection data filter Added command line option to disable filter criteria (other viruses)
Fix unsupported operand type(s) for +=: 'dict' and 'int' error in ena accessor
Fix error in test data trying to insert the samples from previous test
Fix yet another typo
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #144 +/- ##
===========================================
+ Coverage 85.87% 85.93% +0.05%
===========================================
Files 48 48
Lines 5026 5047 +21
===========================================
+ Hits 4316 4337 +21
Misses 710 710
☔ View full report in Codecov by Sentry. |
johausmann
changed the title
|
This addresses the ENA accessor part related to this issue. I would like to discuss the retroactive removal in our next technical meeting. |
| @@ -191,6 +195,12 @@ def _parse_ena_run(self, run): | |||
| def _complies_with_inclusion_criteria(self, ena_run: dict): | |||
| # NOTE: this uses the original dictionary instead of the parsed SampleEna class for performance reasons | |||
| included = True | |||
| # Skip samples with empty collection date - Note this checks not if collection date is too early | |||
| if not self.disable_collection_date_filter: | |||
There was a problem hiding this comment.
This is just a style comment that I normally try to avoid as for me it makes the code slightly more difficult to understand.
Double negations. When interpreting the code in your head and going through the logic, double negations are very error prone (for me, I did not run a trial on it). Here you do not and disable which means enable, so I would call the variable here enable. I understand that this comes from CLI where you want your default to be enabled and hende the flag is disabled, you can keep disable in the CLI.
You don't need to apply this change.
Addresses issue #86