Skip to content

Fix: Staging area validator incorrectly fails on supplementary LungMAP file#27

Merged
ncalvanese1 merged 2 commits into
mainfrom
dsotirho-ucsc/supplementary-file-cgm-validation
Oct 29, 2025
Merged

Fix: Staging area validator incorrectly fails on supplementary LungMAP file#27
ncalvanese1 merged 2 commits into
mainfrom
dsotirho-ucsc/supplementary-file-cgm-validation

Conversation

@dsotirho-ucsc
Copy link
Copy Markdown
Collaborator

@github-actions github-actions Bot added the orange [process] Done by the Azul team label Oct 21, 2025
@dsotirho-ucsc dsotirho-ucsc force-pushed the dsotirho-ucsc/supplementary-file-cgm-validation branch from fb8d33d to 96db216 Compare October 21, 2025 16:42
@JoshuaFortriede
Copy link
Copy Markdown

While this code technically works, I would not recommend it. It appears to me that it will search for "matrix" within any of the values of the content_description dictionary. For ontology fields, there are several valid terms that contain the word matrix, but that are not data:2082. The "text" field is the only field required and this could be anything, such as "My favorite Ice Cream matrix". Finally, the specification dictates a requirement of the content description "is set to"
{
"text": "Contributor-generated matrix",
"ontology": "data:2082",
"ontology_label": "Matrix"
}

So, even a value of the following should not trigger the strata check:
{
"text": "Matrix",
"ontology": "data:2082",
"ontology_label": "Matrix"
}

I would recommend re-working the code to check for the specific structure that is outlined in the DCP/2 System Design documentation.

@ncalvanese1
Copy link
Copy Markdown
Collaborator

From email conversations, Hannes mentioned that "This is the same condition that Azul applies, for historical reasons. We believe it's more important for the validator to be consistent with Azul, than to prevent the scenario you are describing." He confirmed with Joshua that this is not a blocker for the current datasets being submitted, and that we can re-address his comment if and when it becomes an issue.

Copy link
Copy Markdown
Collaborator

@ncalvanese1 ncalvanese1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See conversation in the comments. Solution should be sufficient for now, and match Azul conditions.

@ncalvanese1 ncalvanese1 requested a review from sahakiann October 29, 2025 17:28
@ncalvanese1 ncalvanese1 merged commit 1cfb29f into main Oct 29, 2025
3 checks passed
@ncalvanese1 ncalvanese1 deleted the dsotirho-ucsc/supplementary-file-cgm-validation branch October 29, 2025 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

orange [process] Done by the Azul team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Staging area validator incorrectly fails on supplementary LungMAP file

5 participants