Feat 309: Updates TARS Mappings #342
Conversation
|
Converting to draft until we clarify some things with @saskiad |
src/aind_metadata_service/models.py
Outdated
| class ViralMaterialInformation(ViralMaterial): | ||
| """Viral Material with Stock Titer from SLIMS""" | ||
|
|
||
| stock_titer: Optional[List[int]] = Field(default=None) |
There was a problem hiding this comment.
why is the stock titer a List?
There was a problem hiding this comment.
In TARS, a user can technically add more than one Titer object (each having a value for "result"). In the example below, you can see the + sign on the top right to add more than one. Right now the stock_titer list is all of these titers (Preferred and not). If we only want the preferred value let me know.
There was a problem hiding this comment.
If we have a list of titers, we need to know what they map to. A single plasmid can only have one titer, so then we need a matching list for the plasmids.
It doesn’t make sense to have a “preferred” titer vs non-preferred, so that suggests to me that some of this is TARS functionality that they are just duplicating between modules. But if multiple titers is a thing for a prep lot, then we have to map plasmids differently than we do. E.g. titer and plasmids need to be linked directly.
There was a problem hiding this comment.
Right now, I'm just returning the Preferred plasmid alias for plasmid_tars_alias. @dyf suggested we update the schema in a later ticket to handle the titers/plasmids list. This PR change the tars endpoint to return a custom model dictionary.
There was a problem hiding this comment.
I think we need to do it right and have the plasmid being it’s own class.
But right now looking at TARS I don’t see a link between plasmids and titer, which means nobody has the information. Can we find an actual example of a prep lot with multiple plasmids. We’re doing all this work to accomodate what I understand is an uncommon (but real) use case and it would really help to have an actual example to work with rather than trying to build for hypotheticals.
There was a problem hiding this comment.
I talked with Aaron. (a) no examples exist in TARS and (b) there will only be one titer because the plasmids are packed in one capsid. So make titer not a list and we’re good to go.
tests/tars/test_client.py
Outdated
| @@ -67,16 +65,18 @@ def setUp(self, mock_credential): | |||
| resource="https://some_resource", | |||
| ) | |||
| tars_virus_identifiers = TarsVirusIdentifiers( | |||
| virus_tars_id="AiV456", | |||
| plasmid_tars_alias="AiP123", | |||
| virus_tars_id="AiP123", | |||
There was a problem hiding this comment.
I know this is for a text, but since virus_tars_id are usualy AiVXXX and plasmid_tars_aliases are usually AiPXXX it would be better to not have the virus_tars_id use AiP here to prevent confusion for someone reading it.
tests/tars/test_mapping.py
Outdated
| virus_tars_id="AiV456", | ||
| plasmid_tars_alias="AiP123", | ||
| tars_virus_identifiers = TarsVirusIdentifiers( | ||
| virus_tars_id="AiP123", |
closes #309