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

fix: tissue_ontology_term_id and cell_type_ontology_term_id fields for 3 new species #1235

Merged
merged 4 commits into from
Jan 31, 2025

Conversation

joyceyan
Copy link
Contributor

@joyceyan joyceyan commented Jan 31, 2025

Reason for Change

there's a few issues here that this PR addresses:

  • we need to make sure that zebrafish / fruit fly / roundworm still accepts valid CL terms for tissue_ontology_term_id, but only when tissue_type == cell culture
  • also, UBERON terms should be supported for tissue_ontology_term_id even if species is zebrafish / fruit fly / roundworm
  • and CL terms should be supported for cell_type_ontology_term_id even if species is zebrafish / fruit fly / roundworm

this PR: https://github.com/chanzuckerberg/single-cell-curation/pull/1231/files introduced some issues with these 3 cases weren't being properly handled with moving the logic to the schema def. so i've moved the organism-specific logic back into the validate script directly.

we will revisit an approach to writing out the validation rules here sometime next week.

Testing

  • adds test coverage for these 3 cases, which was missing earlier

Notes for Reviewer

@joyceyan joyceyan changed the title fix: cell culture edge case [WIP] fix: tissue_ontology_term_id and cell_type_ontology_term_id fields for 3 new species Jan 31, 2025
@joyceyan joyceyan requested a review from ejmolinelli January 31, 2025 17:38
Copy link

codecov bot commented Jan 31, 2025

Codecov Report

Attention: Patch coverage is 95.12195% with 2 lines in your changes missing coverage. Please review.

Project coverage is 89.85%. Comparing base (8112a32) to head (22346f4).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1235      +/-   ##
==========================================
+ Coverage   89.79%   89.85%   +0.05%     
==========================================
  Files          19       19              
  Lines        2147     2188      +41     
==========================================
+ Hits         1928     1966      +38     
- Misses        219      222       +3     
Components Coverage Δ
cellxgene_schema_cli 90.79% <95.12%> (+0.05%) ⬆️
migration_assistant 91.26% <ø> (ø)
schema_bump_dry_run_genes 79.80% <ø> (ø)
schema_bump_dry_run_ontologies 99.53% <ø> (ø)

Copy link
Contributor

@ejmolinelli ejmolinelli left a comment

Choose a reason for hiding this comment

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

looks good.

@joyceyan joyceyan merged commit 0387a19 into main Jan 31, 2025
14 checks passed
@joyceyan joyceyan deleted the joyce/cell-culture branch January 31, 2025 17:46
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