Conversation
|
Two suggestions:
|
gouwens
left a comment
gouwens
left a comment
There was a problem hiding this comment.
Overall looks good - just a question about mating status being required or optional.
dbirman
left a comment
dbirman
left a comment
There was a problem hiding this comment.
Need to add a validator for HumanSubject.species to avoid crashing old code (unless we're sure none of these assets exist)
| class HumanSubject(DataModel): | ||
| """Description of a human subject""" | ||
|
|
||
| species: Species.ONE_OF = Field(..., title="Species") |
There was a problem hiding this comment.
I feel like there's a sensible default we could put here. More seriously this is a breaking change, we should add a field_validator in mode="before" that pre-fills this if it's not present to avoid crashing old assets (if any have HumanSubject, I'm not sure they do though)
There was a problem hiding this comment.
It gave me all sorts of errors when I tried to make it a default and I didn't have the patience to figure out why. But yes, I agree.
I think there is one very old exaspim asset with human data, that may or may not have any of the rest of its metadata...I'll look into it. But yes, let's not break everything.
3 participants
closes #1701