-
Notifications
You must be signed in to change notification settings - Fork 127
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
Refactor create_seqinfo tiny bit to avoid duplication and add logging; and in tests to reuse list of dicom paths #785
Conversation
…file at module level I think it is safer to avoid 3rd party library calls at the module import time since if they are buggy it would be too loud of kaboom. Also it adds CPU time at import time even if not used later on. So replaced with defining a list of all DICOMs and then just using that function directly from filename. This also would run it against more DICOMs as this time we would glob recursively.
…we extracted sequence_name
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #785 +/- ##
==========================================
+ Coverage 82.13% 82.15% +0.02%
==========================================
Files 42 42
Lines 4226 4226
==========================================
+ Hits 3471 3472 +1
+ Misses 755 754 -1 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
apart from missing typing, LGTM
Co-authored-by: Basile <[email protected]>
🚀 PR was released in |
Follow up to
triggered by desire to avoid "growing" duplication and wanting to avoid call outs to
wrapper_from_file
at the test module import time.