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

[MODEL] Update data model to include longitudinal pheno data when sessions exist in TSV #250

Merged
merged 8 commits into from
Dec 12, 2023

Conversation

alyssadai
Copy link
Contributor

Changes proposed in this pull request:

  • Created subclasses of the "Session" class -> PhenotypicSession and ImagingSession
  • Updated logic for bagel pheno command to populate attributes for every session that exists in the input phenotypic TSV
  • Updated the existing tests to check at the appropriate level of depth in the output JSONLD files for variables
  • Regenerated the reference example_synthetic JSON and JSONLD following the new session-level phenotypic model
  • Updated the bagel bids command tests to look at only imaging sessions in the output
  • Added example17 which includes no phenotypic sessions (session_id column) in the phenotypic TSV (but this is currently not tested by any tests)

Checklist

  • PR has an interpretable title with a prefix ([ENH], [FIX], [REF], [TST], [CI], [MNT], [INF], [MODEL], [DOC]) (see https://neurobagel.org/contributing/pull_requests for more info)
  • PR links to GitHub issue with mention Closes #XXXX
  • Tests pass
  • Checks pass

For new features:

  • Tests have been added

For bug fixes:

  • There is at least one test that would fail under the original bug conditions.

alyssadai and others added 6 commits December 11, 2023 13:56
Co-written by
- Pinning
- Sam
- Alyssa
- Seb
- Emille
- Naga
Co-authored-by: Alyssa Dai <[email protected]>
Co-authored-by: sam-gregz <[email protected]>
Co-authored-by: Renee He <[email protected]>
Co-authored-by: nagatv11 <[email protected]>
Co-authored-by: Alyssa Dai <[email protected]>
Co-authored-by: sam-gregz <[email protected]>
Co-authored-by: Renee He <[email protected]>
Co-authored-by: nagatv11 <[email protected]>
@alyssadai alyssadai requested a review from surchs December 12, 2023 17:05
Copy link

github-actions bot commented Dec 12, 2023

Pull Request Test Coverage Report for Build 7184951017

  • 70 of 70 (100.0%) changed or added relevant lines in 4 files are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.004%) to 99.69%

Files with Coverage Reduction New Missed Lines %
bagel/tests/conftest.py 2 88.57%
Totals Coverage Status
Change from base Build 7177602160: 0.004%
Covered Lines: 1286
Relevant Lines: 1290

💛 - Coveralls

bagel/cli.py Outdated Show resolved Hide resolved
Copy link
Contributor

@surchs surchs left a comment

Choose a reason for hiding this comment

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

what an excellent example of pair programming 👯

🧑‍🍳

bagel/cli.py Outdated Show resolved Hide resolved
Co-authored-by: Sebastian Urchs <[email protected]>
@alyssadai alyssadai merged commit e06a440 into main Dec 12, 2023
6 checks passed
@alyssadai alyssadai deleted the adai-pheno-layer branch December 12, 2023 17:18
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