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

C elegens metadata #353

Merged
merged 10 commits into from
May 3, 2023
Merged

C elegens metadata #353

merged 10 commits into from
May 3, 2023

Conversation

bendichter
Copy link
Contributor

Motivation

From conversation with Daniel Sprague:

I was wondering if there were possible changes we could make to the restrictions imposed on age and sex. C. elegans growth stages are defined from L1-L4 followed by adulthood. Most of the worms we use are designated 'YA' for young adult. I believe for our purposes, this is a more useful designation than dahs or weeks old since the lifesplan of the owrm is relatively short. Furthermore, for sex, C. elegans have two sexes: male 'XO' and hermaphrodite 'XX' which are designated by their sex chromosomes. I see that there is an option for other, but since the hermaphrodite is the typical system we study, I think it would be useful to be able to just input the chromosomal designations if that's possible.

What do you think, @CodyCBakerPhD ?

@CodyCBakerPhD
Copy link
Collaborator

XX and XO appear to be officially designated for this species, so yes perfectly fine by me

@CodyCBakerPhD
Copy link
Collaborator

The other life cycle stuff is a part of the ongoing discussion for handling age: #209

@CodyCBakerPhD
Copy link
Collaborator

Just need a couple tests and this is good to go

@CodyCBakerPhD
Copy link
Collaborator

@rly I remember you mentioning something about the reasons the DANDI tests are failing - can you confirm there is no easy way to fix the CI here?

@rly
Copy link
Contributor

rly commented Mar 29, 2023

The tests fail because dandi raises DeprecationWarnings into errors. We have a fix in the dev branch of PyNWB that we haven't released yet. I'm getting to it...

@CodyCBakerPhD
Copy link
Collaborator

The tests fail because dandi raises DeprecationWarnings into errors. We have a fix in the dev branch of PyNWB that we haven't released yet. I'm getting to it...

Thanks for confirming, no huge rush on my end I just wanted to know for sure

@codecov-commenter
Copy link

Codecov Report

Merging #353 (c8c595a) into dev (c5e9770) will decrease coverage by 0.16%.
The diff coverage is 33.33%.

❗ Current head c8c595a differs from pull request most recent head 39f0880. Consider uploading reports for the commit 39f0880 to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##              dev     #353      +/-   ##
==========================================
- Coverage   91.13%   90.98%   -0.16%     
==========================================
  Files          20       20              
  Lines        1117     1120       +3     
==========================================
+ Hits         1018     1019       +1     
- Misses         99      101       +2     
Flag Coverage Δ
unittests 90.98% <33.33%> (-0.16%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/nwbinspector/checks/nwbfile_metadata.py 97.24% <33.33%> (-1.81%) ⬇️

@CodyCBakerPhD CodyCBakerPhD marked this pull request as ready for review April 24, 2023 18:06
@CodyCBakerPhD
Copy link
Collaborator

@rly Good to merge?

@CodyCBakerPhD CodyCBakerPhD requested a review from rly April 24, 2023 18:06
@CodyCBakerPhD CodyCBakerPhD self-requested a review May 3, 2023 15:28
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.

4 participants