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] Filter for only ImagingSessions or PhenotypicSessions in SPARQL query #375

Merged
merged 7 commits into from
Nov 7, 2024

Conversation

alyssadai
Copy link
Contributor

@alyssadai alyssadai commented Nov 6, 2024

Changes proposed in this pull request:

  • Update query template to exclude instances of Session when matching sessions (returned by default due to RDF inferencing based on class relationships)
  • Run integration tests using a test graph as part of CI

Checklist

This section is for the PR reviewer

  • PR has an interpretable title with a prefix ([ENH], [FIX], [REF], [TST], [CI], [MNT], [INF], [MODEL], [DOC]) (see our Contributing Guidelines for more info)
  • PR has a label for the release changelog or skip-release (to be applied by maintainers only)
  • PR links to GitHub issue with mention Closes #XXXX
  • Tests pass
  • Checks pass
  • If the PR changes the SPARQL query template, the default Neurobagel query file has also been regenerated

For new features:

  • Tests have been added

For bug fixes:

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

- RDFS inference will also return instances of the parent Session by default
@alyssadai alyssadai added release Create a release when this PR is merged pr-patch Incremental feature improvement, will increment patch version when merged (0.0.+1) labels Nov 6, 2024
@coveralls
Copy link
Collaborator

coveralls commented Nov 6, 2024

Pull Request Test Coverage Report for Build 11712481091

Details

  • 11 of 11 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.4%) to 97.052%

Totals Coverage Status
Change from base Build 11672979429: 0.4%
Covered Lines: 823
Relevant Lines: 848

💛 - Coveralls

@alyssadai alyssadai marked this pull request as ready for review November 6, 2024 22:01
Copy link
Collaborator

@rmanaem rmanaem left a comment

Choose a reason for hiding this comment

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

🧑‍🍳

@alyssadai alyssadai merged commit fd11d86 into main Nov 7, 2024
8 checks passed
@alyssadai alyssadai deleted the fix-session-matching branch November 7, 2024 16:07
Copy link
Contributor

neurobagel-bot bot commented Nov 7, 2024

🚀 PR was released in v0.4.2 🚀

@neurobagel-bot neurobagel-bot bot added the released This issue/pull request has been released. label Nov 7, 2024
@alyssadai
Copy link
Contributor Author

Note that our public nodes need to be redeployed with this fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-patch Incremental feature improvement, will increment patch version when merged (0.0.+1) release Create a release when this PR is merged released This issue/pull request has been released.
Projects
None yet
3 participants