Skip to content

Preserve NWB subject age-reference metadata - #1931

Merged
yarikoptic merged 4 commits into
dandi:masterfrom
AtomicGlance:fix/1242-age-reference
Oct 1, 2026
Merged

yarikoptic merged 4 commits into
dandi:masterfrom
AtomicGlance:fix/1242-age-reference

Conversation

@AtomicGlance

Copy link
Copy Markdown
Contributor

Fixes #1242.

NWB subjects can store a gestational age as an ordinary duration such as P3W, with its meaning supplied by age__reference. That reference was not included in the extracted metadata, so the duration became birth-referenced.

This includes age__reference in subject metadata and uses a gestational reference when converting the age. Existing gestational-prefix strings remain supported. When age is calculated from date of birth and session start, it remains birth-referenced; that calculation is unchanged.

Tests cover birth and gestational references through an NWB write/read round trip, legacy strings and missing references, and date-of-birth precedence.

Local validation on Windows/Python 3.13:

  • Metadata tests excluding BIDS, ontology/network and remote-asset cases: 153 passed, 2 existing expected-failure tests unexpectedly passed, 11 deselected.
  • Black, isort and flake8 passed.
  • mypy dandi: no issues in 96 source files.

The full integration suite has not been run locally.

@codecov

codecov Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.40%. Comparing base (eb37fdb) to head (26cec27).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1931      +/-   ##
==========================================
+ Coverage   78.34%   78.40%   +0.06%     
==========================================
  Files          92       92              
  Lines       14109    14142      +33     
==========================================
+ Hits        11053    11088      +35     
+ Misses       3056     3054       -2     
Flag Coverage Δ
unittests 78.40% <100.00%> (+0.06%) ⬆️

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

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@AtomicGlance

Copy link
Copy Markdown
Contributor Author

Could a maintainer please add the patch label to this PR and #1932 when convenient? Both are bug fixes, and their label checks are failing because no release-category label is set. Thank you!

Comment thread dandi/tests/test_metadata.py Outdated
@AtomicGlance

Copy link
Copy Markdown
Contributor Author

some relevant (and not finished :-/ ) work in BIDS and near:

* [Formalize participants' age to clarify the reference point bids-standard/bids-specification#1634](https://github.com/bids-standard/bids-specification/issues/1634)

* [[ENH] Additional age related columns for participants.tsv file to cover animal data bids-standard/bids-specification#1839](https://github.com/bids-standard/bids-specification/pull/1839)

* [[ENH] Additional age related columns for participants.tsv file to cover animal data contd. bids-standard/bids-specification#2340](https://github.com/bids-standard/bids-specification/pull/2340)

* [Add AgeReference for non-birth based age con/nwb2bids#420](https://github.com/con/nwb2bids/issues/420)

Thanks for sharing these, Yaroslav! It’s helpful to see how this connects to the work on age references in BIDS, and I’m glad the PR sparked the nwb2bids discussion too. I’ll read through the proposals to get a better understanding.

@AtomicGlance

Copy link
Copy Markdown
Contributor Author

some relevant (and not finished :-/ ) work in BIDS and near:

* [Formalize participants' age to clarify the reference point bids-standard/bids-specification#1634](https://github.com/bids-standard/bids-specification/issues/1634)

* [[ENH] Additional age related columns for participants.tsv file to cover animal data bids-standard/bids-specification#1839](https://github.com/bids-standard/bids-specification/pull/1839)

* [[ENH] Additional age related columns for participants.tsv file to cover animal data contd. bids-standard/bids-specification#2340](https://github.com/bids-standard/bids-specification/pull/2340)

* [Add AgeReference for non-birth based age con/nwb2bids#420](https://github.com/con/nwb2bids/issues/420)

Thanks for sharing these, Yaroslav! It’s helpful to see how this connects to the work on age references in BIDS, and I’m glad the PR sparked the nwb2bids discussion too. I’ll read through the proposals to get a better understanding.

Hi @yarikoptic, could we move forward with this? Is there anything else you’d like me to add? All checks are passing.

I’m also working on another issue, but I can’t open the PR, it’s stuck in drafts because GitHub says I have too many open issues. 😅 Any advice on how to proceed?

@yarikoptic yarikoptic added the minor Increment the minor version when merged label Oct 1, 2026

@yarikoptic yarikoptic left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it is good (enough), and would be great to see effects in the wild ;-)

@yarikoptic
yarikoptic merged commit 563f2d7 into dandi:master Oct 1, 2026
38 of 39 checks passed
@yarikoptic

Copy link
Copy Markdown
Member

I’m also working on another issue, but I can’t open the PR, it’s stuck in drafts because GitHub says I have too many open issues. 😅

how do I get such a feature to keep my "event horizon" observable? ;-)

@AtomicGlance

Copy link
Copy Markdown
Contributor Author

I’m also working on another issue, but I can’t open the PR, it’s stuck in drafts because GitHub says I have too many open issues. 😅

how do I get such a feature to keep my "event horizon" observable? ;-)

Lmao, if you find a way to keep the GitHub event horizon observable, please share it, I could use that feature too. 😄

Thank you for the thoughtful review and for merging this. I’m excited to see how the age reference change works with real datasets. I’m hoping to become a Dartmouth student someday; perhaps we’ll get to compare our GitHub queues as colleagues in person😅

@yarikoptic

Copy link
Copy Markdown
Member

Lmao, if you find a way to keep the GitHub event horizon observable, please share it, I could use that feature too. 😄

well, since you asked... @CodyCBakerPhD of our https://github.com/con created https://historia.readthedocs.io/ which we are adopting wider within our group (there it says it is a "command-line tool") but the idea is not to see CLI but rather work with github projects. We keep them private ATM, but I guess I could give you a sneak preview of mine although I frankly yet to start using it consciously (need to just automate more) but already expecting from the others ;-)

image

@AtomicGlance

Copy link
Copy Markdown
Contributor Author

Lmao, if you find a way to keep the GitHub event horizon observable, please share it, I could use that feature too. 😄

well, since you asked... @CodyCBakerPhD of our https://github.com/con created https://historia.readthedocs.io/ which we are adopting wider within our group (there it says it is a "command-line tool") but the idea is not to see CLI but rather work with github projects. We keep them private ATM, but I guess I could give you a sneak preview of mine although I frankly yet to start using it consciously (need to just automate more) but already expecting from the others ;-)

image

Ooooh, that's so cool would love to see it in action.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

minor Increment the minor version when merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

parse age reference

2 participants