moved QC into current schema #999
Conversation
|
still don't understand these error messages. Most of them are for files this PR doesn't touch. The one about these files is uninterpretable. |
|
TODO: remove |
|
|
||
| FAIL = "Fail" | ||
| PASS = "Pass" | ||
| UNKNOWN = "Unknown" |
There was a problem hiding this comment.
do we need unknown? I think this it is trying to fill a role like "comment" in on a github PR, where you want to make a note but don't want to pass judgement.
There was a problem hiding this comment.
I expect we're going to add a bunch of other options to this.
| evaluation_modality: Modality.ONE_OF = Field(..., title="Modality") | ||
| evaluation_stage: str = Field(..., title="Evaluation stage") | ||
| evaluator_full_name: str = Field(..., title="Evaluator full name") | ||
| evaluation_date: date = Field(..., title="Evaluation date") |
| describedBy: str = Field(_DESCRIBED_BY_URL, json_schema_extra={"const": _DESCRIBED_BY_URL}) | ||
| schema_version: Literal["0.0.1"] = Field("0.0.1") | ||
| overall_status: Status = Field(..., title="Overall status") | ||
| overall_status_date: date = Field(..., title="Date of status") |
There was a problem hiding this comment.
I still think date.
|
@jtyoung84 I am good with this for now, it will change. But it's a new core schema so want your eyes on it. |
| evaluator_full_name: str = Field(..., title="Evaluator full name") | ||
| evaluation_date: date = Field(..., title="Evaluation date") | ||
| qc_metrics: AindGenericType = Field(AindGeneric(), title="QC metrics") | ||
| stage_status: Status = Field(..., title="Stage status") |
There was a problem hiding this comment.
Since this a required field, do we want an Unknown option? Will it always be Pass/Fail?
There was a problem hiding this comment.
you'll need to duke that out with @dyf - he removed the Unknown option. My hunch is that we're going to end up with a handful of Status options very quickly, but I don't think we want one to be Unknown.
There was a problem hiding this comment.
My worry is, that since it's required, if we ever upgrade old metadata records, we'll just fall back to using QCEvaluation.model_construct() everywhere, which is frustrating.
|
Do you want to add this in the core.Metadata class also? It won't show up in DocDB otherwise. |
|
Should we also add something in the examples folder? |
Yes |
yes |
|
I can help adding it to the core.Metadata class once everything else is resolved |
No description provided.