neurobagel / bagel-cli

Command line tool for Neurobagel data parsing and annotation
https://neurobagel.org/cli/
MIT License
2 stars 5 forks source link

[REF] Refactor utilities #385

Closed alyssadai closed 2 weeks ago

alyssadai commented 2 weeks ago

Changes proposed in this pull request:

Checklist

This section is for the PR reviewer

For new features:

For bug fixes:

codecov[bot] commented 2 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 98.36%. Comparing base (e93423d) to head (e387f06). Report is 11 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #385 +/- ## ========================================== - Coverage 98.37% 98.36% -0.01% ========================================== Files 16 21 +5 Lines 982 980 -2 ========================================== - Hits 966 964 -2 Misses 16 16 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.


🚨 Try these New Features:

coveralls commented 2 weeks ago

Pull Request Test Coverage Report for Build 11784441718

Details


Totals Coverage Status
Change from base Build 11716298122: -0.004%
Covered Lines: 964
Relevant Lines: 980

💛 - Coveralls
alyssadai commented 2 weeks ago

Hey @surchs, just to reply to your concern regarding the removed test:

I think we already had the case that we didn't include a class in the context and that then breaks things in confusing ways. So I don't think removing the test completely is a solution either

To clarify, we added two new tests in PR #349, test_all_used_namespaces_have_urls and test_used_namespaces_in_context, that should catch these issues without requiring us to remember to update the test parameters whenever we update the data model! :) The removed test was just leftover from a #TODO in that PR.

neurobagel-bot[bot] commented 1 week ago

:rocket: PR was released in v0.3.4 :rocket: