Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add uberon_id check for tissue #371

Merged
merged 6 commits into from
Mar 13, 2025
Merged

Conversation

sarahgonicholson
Copy link
Contributor

@sarahgonicholson sarahgonicholson commented Mar 7, 2025

  • Add property valid_protocol_ids to OntologyTerm that is a list of protocol IDs (e.g. 1D, 3A), the portion of external_id indicating tissue types defined here
  • Add a check to Tissue ensuring that the protocol ID in external_id is among the valid_protocol_ids for the OntologyTerm linked with uberon_id
  • Add checks to Donor and Tissue ensuring that external_id matches expected format for Benchmarking and Production
  • Makes external_id a required property for Donor

Copy link
Contributor

@aschroed aschroed left a comment

Choose a reason for hiding this comment

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

I think this looks fine though I wonder how controlled the external_id is. It appears to assume a hyphen separating 2 'meaningful' values? From what I can tell the schema nor tissue validators check that this is the case? Maybe it's there and I missed it as things are so spread around in utils etc. Probably not a big deal but seems like there could be possibility of exceptions being thrown by some of the utils functions if for example there is not a hyphen.

def get_valid_protocol_ids(properties: Dict[str, Any]) -> str:
"""Get valid_protocol_ids from properties."""
return properties.get("valid_protocol_ids","")
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't the default get result be an empty array?

@sarahgonicholson
Copy link
Contributor Author

@aschroed Thanks for your comments! That is a great point. I added checks to Tissue and Donor to make sure that external_id is in the expected format for Benchmarking and Production tissues

@sarahgonicholson sarahgonicholson merged commit b90ee12 into main Mar 13, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants