Conversation
Added Diffuse Large B Cell Lymphoma to the controlled vocabulary for disease_term as required for the PRECINCT02 study Changed data type of the physical_exam: assessment_timepoint property to string and added the appropriate property description Modified various properties with the file node: file_description file_size md5sum
Added values for sample_site Added a new value for specific_sample_pathology Added "Not Determined" as an acceptable value for tumor_grade
Updated the order in which properties are specified within certain nodes to impose more logical and/or indicate importance in our data loading templates Added guidance as to the correct use of relatively similar acceptable terms within a number of controlled vocabularies to the appropriate property descriptions such that the guidance will be visible to the end user, and removed the corresponding comments where that guidance had previously resided out of sight Trimmed out various other comments
Tidied up markers as to where guidance on use of acceptable terms had been added
…port of the file manifest
Changed date_of_iacuc_approval from datetime to string in order to accommodate studies having more than one IACUC approval date, with the OSA02 study being a prime example of that situation
Add new file to support content and content order for download and ex…
General maintenance updates
CRDCDH-1308
CRDCDH-1308
This reverts commit b0bd63b.
CRDCDH-1308
added missing fields for pi_id
add corresponding display values for properties in manifest YAML
Remove duplicate props from icdc-manifest-props.yml
add test property (to be reverted)
Revert "add test property (to be reverted)"
to comply with the spec
add cohort node and move props (manifest YAML)
Expanded controlled vocabularies for the following properties: primary_disease_site sample_site physical_sample_type summarized_sample_type specific_sample_pathology file_type
Changed datatype of "Integer" to "integer" on the following three properties: disease_extent: evalutaion_number disease_extent: lesion_number visit: visit_number This brings in the same changes that were PR'ed by Mark Jensen as of 11-12-2024, but which never got merged in.
…dies Fix #168 Update model per STS01 and OSA04 requirements
Updated the controlled vocabulary for the file_type property putting acceptable terms into alphabetical order; existing order was entirely arbitrary and was being propagated as is into the Data Model Navigator, where it really should be presented in a logical order.
Unrelated to either the STS01 or the OSA04 studies: Changed cardinality of the relationship between diagnosis and case from many_to_one to one_to_one Updated the dose_limiting_toxicity and unexpected_adverse_event properties with the adverse_event node from Required to Preferred, in order to accommodate the blanks values we're seeing in the data generated by the PRECINCT01 and PRECINCT02 studies
Changed: uses: CBIIT/bento-workflows/.github/workflows/model-test-and-deploy.yml@v<X>.<Y>.<Z> from uses: CBIIT/bento-workflows/.github/workflows/model-test-and-deploy.yml@v2.1.7 to uses: CBIIT/bento-workflows/.github/workflows/model-test-and-deploy.yml@v2.3.12
…dies Fix #168 Update Model per STS01 and OSA04 requirements - polish
ICDC-3883
Maj val fix
CRDCDH-1344
PhilipMusk
left a comment
PhilipMusk
left a comment
There was a problem hiding this comment.
Like wow scoobs.....
Aside from some of Eric's commits and merges of changes made for the purposes of demonstrating the utility of the manifest props file which were to be reverted back out, I think I've reviewed literally every commit in this masterpiece.
The first commit to it being dated October 31, 2023 is both ironic and entirely fitting.
Everything I reviewed eventually made sense. I say eventually because some of the early commits of the manifest props file has me a little confused by some duplication that I noted and expected to see resolved before it actually was. In addition to which, much of the evolution of the manifest props file happened while I was somewhat disengaged from ICDC development work. That said, the final iterations of the manifest props file made complete sense. That third file is very cleverly put together.
Concerning the deprecation of the image and the agent administration nodes, I would have to say good riddance.
On the image collection node, I noted its deprecation, followed by its re-instatement; however I do still have one question about the node as it stands. That being its lack of anything akin to image_collection_record_id as its key property. I think image_collection_name was and perhaps still is its key property. But I have the CRDC Submission Portal on Line 1 asking about a record ID property.
I for one will be glad when this monster is finally merged in, and we can pick up the pieces downstream of it. Submission Portal compatibility changes aside, there's actually a ton of really good stuff in this merge. Let's just do this and get on with our lives.
PhilipMusk
left a comment
PhilipMusk
left a comment
There was a problem hiding this comment.
Having already reviewed this PR in detail, and having provided some feedback, I'm approving the changes in question.
4 participants
No description provided.