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.

Sign up for GitHub

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

Jump to bottom

[ENH] Add all possible cases of missing values to example_synthetic #148

Merged
merged 8 commits into from
Jun 9, 2023
Merged

[ENH] Add all possible cases of missing values to example_synthetic #148

merged 8 commits into from
Jun 9, 2023

Conversation

alyssadai
Copy link
Contributor

@alyssadai alyssadai commented Jun 6, 2023 •
edited by jarmoza
Loading

Closes #144

Changes proposed in this pull request:

  • Missing value annotations + entries added for Age, Sex, and Diagnosis in the example_synthetic data files
  • pheno command updated to only create + assign instances of Sex when values aren't missing
  • Added a sample "final" .jsonld file containing both pheno + BIDS metadata for example_synthetic
  • Updated test data README with descriptions of available .jsonld examples

Checklist

  • PR has an interpretable title with a prefix ([ENH], [BUG], [DOC], [INFRA], [MAINT])
  • PR links to Github issue with mention Closes #XXXX
  • Tests pass
  • Code is properly formatted

@coveralls
Copy link
Collaborator

coveralls commented Jun 6, 2023 •
edited by github-actions bot
Loading

Pull Request Test Coverage Report for Build 5223031031

  • 11 of 11 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.02%) to 99.121%
Totals Coverage Status
Change from base Build 5183532917: 0.02%
Covered Lines: 451
Relevant Lines: 455
💛 - Coveralls

@jarmoza jarmoza self-requested a review June 8, 2023 15:06
jarmoza
jarmoza suggested changes Jun 8, 2023
View reviewed changes
Copy link
Contributor

@jarmoza jarmoza left a comment

Choose a reason for hiding this comment

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

@alyssadai Could you add a new test for this new missing values / 'Sex'-categorized column scenario?

bagel/cli.py Show resolved Hide resolved
bagel/tests/data/README.md Show resolved Hide resolved
@alyssadai alyssadai changed the title Add all possible cases of missing values to example_synthetic [ENH] Add all possible cases of missing values to example_synthetic Jun 9, 2023
@alyssadai alyssadai requested a review from jarmoza June 9, 2023 14:31
jarmoza
jarmoza approved these changes Jun 9, 2023
View reviewed changes
Copy link
Contributor

@jarmoza jarmoza left a comment

Choose a reason for hiding this comment

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

Thanks, @alyssadai. Looks good! 🧑‍🍳

@alyssadai alyssadai merged commit dd54f14 into main Jun 9, 2023
@alyssadai alyssadai deleted the enh-144/update_ref_example branch June 9, 2023 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jarmoza jarmoza jarmoza approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[ENH] Update example_synthetic.json to be a comprehensive reference example
3 participants
@alyssadai @coveralls @jarmoza