Add section to reports that show system info, tedana call and version

[eurunuela]

Closes #650 and #687 .

Changes proposed in this pull request:

• Add a function to return system info, then save it into a json file.
• Save tedana call into a variable and add to the dictionary with system info.
• Add tedana version into the dictionary with system info.
• Get Python version and add it to the dictionary with system info.

[eurunuela]

This PR is pretty much ready for review but I would like to wait until #696 is merged to make some final changes for the sake of coherence.

[jbteves]

@eurunuela this looks good but there's actually one potential downfall: if you call tedana with the API instead of on the command line, I'm not sure you would get a good idea of what the API was called with. We may instead want to create a list of parameters that get captured from the API call and display those. We could also add some logic by having a new workflow variable that captures the mode as cli at the __name__ == '__main__' check and add logic to this PR to format appropriately for CLI/API. Happy to help with that.

[tsalo]

We may instead want to create a list of parameters that get captured from the API call and display those. We could also add some logic by having a new workflow variable that captures the mode as cli at the __name__ == '__main__' check and add logic to this PR to format appropriately for CLI/API.

I would be pretty happy with just a list/table/dict of parameters, rather than a reconstructed CLI call. Just my personal preference though.

[eurunuela]

@tsalo @jbteves do you like it now?

[codecov]

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.14% 🎉

Comparison is base (bf3392a) 88.86% compared to head (83ffeff) 89.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #747      +/-   ##
==========================================
+ Coverage   88.86%   89.00%   +0.14%     
==========================================
  Files          27       27              
  Lines        3368     3411      +43     
  Branches      618      622       +4     
==========================================
+ Hits         2993     3036      +43     
  Misses        227      227              
  Partials      148      148              

Files Changed	Coverage Δ
tedana/reporting/html_report.py	92.59% <100.00%> (+1.19%)	⬆️
tedana/utils.py	94.73% <100.00%> (+0.14%)	⬆️
tedana/workflows/ica_reclassify.py	97.92% <100.00%> (+0.13%)	⬆️
tedana/workflows/tedana.py	81.81% <100.00%> (+0.86%)	⬆️

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

[jbteves]

I'm not entirely sure we want to display it CLI-style instead of printing a dict of parameters, but I'm guessing that it's easier to make pretty if we do it CLI-style and it should still be plenty legible. Plus, people could just copy and paste it to replicate.

[eurunuela]

I'm not entirely sure we want to display it CLI-style instead of printing a dict of parameters, but I'm guessing that it's easier to make pretty if we do it CLI-style and it should still be plenty legible. Plus, people could just copy and paste it to replicate.

I believe the way I put it it won't be exactly the same as the CLI but it should be pretty close.

[tsalo]

Here's what it currently looks like for the four-echo test:

I really like it. I see that there are some issues with unset arguments (e.g., --prefix followed by nothing, --mixm None) that make me think that a table might be better, but it's still pretty interpretable. We may need a warning for naive users though. Just something telling them not to just copy and paste the command, since it's not exact.

One minor thing I'd love to see is a section title for the methods description, so that it's clear what the boundary is between command/system info and the report boilerplate.

[eurunuela]

Hey @tsalo @handwerkerd, you might have missed the notification for this one.

This PR is ready for review now. Let me know if you would make any changes.

[tsalo]

Could you copy over the changes to .circleci/config.yml from #949 so we can look at the test outputs as well? It'll make it easier to check over the effects of your changes.

[eurunuela]

Could you copy over the changes to .circleci/config.yml from #949 so we can look at the test outputs as well? It'll make it easier to check over the effects of your changes.

Done in 11b9701.

[tsalo]

Thanks for this @eurunuela! I like the formatting of the system info, but I do think the command formatting could be improved. That said, we can always tweak the style in a later PR.

One unfortunate thing is that the "command" shows a test script when one is used. See the "command" for the three-echo test:

tedana --log-cli-level=INFO --cov-append --cov-report term-missing --cov=tedana -k test_integration_three_echo tedana/tests/test_integration.py

I don't think there's a way to get around this easily. It's probably just a limitation we'll have to acknowledge.

Regardless, everything looks really awesome.

[eurunuela]

How about this new design for the code block @tsalo?

[tsalo]

The new design looks great! Thanks @eurunuela!

[handwerkerd]

This looks great.

I'm approving, but I suggested one small change that you can revise or ignore.

One additional thing to consider is to add the version numbers of python modules that are most likely to affect results: mapca, nilearn, scipy, numpy, pandas, sklearn

[handwerkerd]

Minor suggestion. We are currently creating several files outside of the io_generator: report, references, & tedana_.log. Since this is another file being created outside of the io_generator, it would fit better near the creation of these other file names (after line 473) instead of after the creation of the io_generator. (longer-term, we might want all of these to get added to the io_generator, but that's not critical)

Also, if this is merged before #963 then add the prefix to tedana_call.sh in #963

[eurunuela]

I have moved the tedana command creation closer to the report, references and log creation.

One additional thing to consider is to add the version numbers of python modules that are most likely to affect results: mapca, nilearn, scipy, numpy, pandas, sklearn

I would leave the addition of these module versions for a future PR.

[handwerkerd]

LGTM

[tsalo]

LGTM