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

Adding python library version numbers #1014

Merged
merged 2 commits into from
Jan 10, 2024

Conversation

handwerkerd
Copy link
Member

@handwerkerd handwerkerd commented Jan 8, 2024

When we closed #747 I had made a comment that we might also want to log the version numbers of key modules used by tedana. This PR does that (plus identifies & corrects an omission from that PR)

Changes proposed in this pull request:

  • dataset_description.json includes the version numbers for all libraries specified in project.toml
  • utils.get_system_info is now utils.get_system_and_version_info. The previous line to get the python version was moved from tedana.py and ica_reclassify.py into this function and this function now compiles the version numbers for the other packages
  • I wouldn't mind someone making sure I'm getting all the version numbers in a pythonic and not-too-inefficient way.
  • In making the above changes, I noticed the tedana and ica_reclassify workflows saves the system information, but t2smap did not so I added that info to t2smap
  • I also noticed that the method in the _main functions for getting the text of a command line input worked for an actual command line input, but not tedana_cli calls, which we were using in the tests. I think I fixed this is a way that won't break anything else.
    • This causes code coverage to drop since the CLI line that created garbled outputs is now skipped. Not sure how to directly call that line.
  • Added library version info to tedana_report.html via edits to report_info_table_template.html and html_report.py

For future work (or edits to this PR). The three work flows all have a very similar block of text to specify and write derivative_metadata This could be it's own function. Also, BIDSVersion is hard-coded in all three. Given 1.5.0 is the only version we support, that's not a big deal, but, if we ever add an updated output option, we might want to make that a variable.

I was reminded to do this via nipreps/fmriprep#3196

@handwerkerd handwerkerd added reports issues related to boilerplate generation or visual reports effort: low Theoretically less than a day's work impact: medium Improves code/documentation functionality for some users maintenance issues related to versioning, dependencies, and other related elements labels Jan 8, 2024
Copy link

codecov bot commented Jan 8, 2024

Codecov Report

Attention: Patch coverage is 83.33333% with 6 lines in your changes missing coverage. Please review.

Project coverage is 89.44%. Comparing base (2525ba3) to head (f218092).
Report is 61 commits behind head on main.

Files with missing lines Patch % Lines
tedana/workflows/ica_reclassify.py 50.00% 1 Missing and 1 partial ⚠️
tedana/workflows/t2smap.py 85.71% 1 Missing and 1 partial ⚠️
tedana/workflows/tedana.py 50.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1014      +/-   ##
==========================================
- Coverage   89.54%   89.44%   -0.10%     
==========================================
  Files          26       26              
  Lines        3395     3420      +25     
  Branches      619      624       +5     
==========================================
+ Hits         3040     3059      +19     
- Misses        207      210       +3     
- Partials      148      151       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@tsalo tsalo left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Collaborator

@eurunuela eurunuela left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you Dan.

@handwerkerd handwerkerd merged commit d9644ed into ME-ICA:main Jan 10, 2024
16 checks passed
@handwerkerd handwerkerd deleted the Adding-lib-version-numbs branch January 10, 2024 19:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort: low Theoretically less than a day's work impact: medium Improves code/documentation functionality for some users maintenance issues related to versioning, dependencies, and other related elements reports issues related to boilerplate generation or visual reports
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants