search

populationgenomics / metamist

Sample level metadata system
MIT License
1 stars 1 forks source link

Refactor all *.from_db() routines to use from_db_json() #821

Open jmarshall opened 4 weeks ago

jmarshall commented 4 weeks ago

Stacked on top of PR #659 (but for merging into dev after that one has been merged), this refactors existing code to use from_db_json(), newly added in the multiple external-ids PR.

This wrapper calls json.loads() but also handles None (returning None), which enables the code at many call sites to be simplified.

Removed some callers' if isinstance(field, str): ... code, which has the effect of newly disallowing field values that are already dicts. However we've verified that all *.from_db() calls have raw database outputs as their arguments, so such fields will be always be strings and IMHO giving a dict to from_db_json() is really a logic error that should be detected.

In SequencingGroupInternal.from_db() added pop(..., None) so that a missing meta field is now accepted. The previous code suggests that having pop() produce KeyError here was unintended.

The expected argument types for from_db_json() are listed in the definition, but we don't list its return type. The best we could say in general is object but most call sites expect dict[str, str] (or occasionally list[str]) due to the shape of their expected JSON. Specifying object would lead to mypy errors at these call sites.

I suppose we could have a dict_from_db_json() routine for the common case of meta dict[str,str] expansion that could have that return type annotation and could even validate that the JSON was of the expected form! But I think that would be overkill…

codecov-commenter commented 4 weeks ago

Codecov Report

Attention: Patch coverage is 97.14286% with 1 line in your changes missing coverage. Please review.

Project coverage is 79.15%. Comparing base (71dddbe) to head (38e7f1a). Report is 1 commits behind head on dev.

Files Patch % Lines
db/python/tables/participant_phenotype.py 66.66% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## dev #821 +/- ## ========================================== + Coverage 79.04% 79.15% +0.11% ========================================== Files 161 161 Lines 13607 13593 -14 ========================================== + Hits 10755 10759 +4 + Misses 2852 2834 -18 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.