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] Eliminate duplicate lines in logs #645

Merged
merged 1 commit into from
Jan 15, 2021

Conversation

tsalo
Copy link
Member

@tsalo tsalo commented Jan 14, 2021

Closes #638.

Changes proposed in this pull request:

  • Add a report argument to relevant functions so that we can toggle whether a given function will add to the report logger. In cases where the same function is called multiple times, we turn the report off.

@codecov
Copy link

codecov bot commented Jan 14, 2021

Codecov Report

Merging #645 (fa6e604) into master (5369e1d) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #645      +/-   ##
==========================================
+ Coverage   93.56%   93.59%   +0.02%     
==========================================
  Files          26       26              
  Lines        1975     1984       +9     
==========================================
+ Hits         1848     1857       +9     
  Misses        127      127              
Impacted Files Coverage Δ
tedana/combine.py 91.17% <100.00%> (+0.85%) ⬆️
tedana/decay.py 95.58% <100.00%> (+0.09%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5369e1d...fa6e604. Read the comment docs.

Copy link
Collaborator

@jbteves jbteves left a comment

Choose a reason for hiding this comment

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

I'm approving because I think this works and I can't think of anything better, but we'll have to be careful about maintaining those blocks of report being True and False. I think it could easily trip us up if we're not careful. But as I said, this still looks good to me!
Thanks @tsalo !

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.

I'm not sure this is the best solution but I get that right now it's a working solution. We should still think of a more elegant way to deal with the duplicate logs.

@tsalo tsalo merged commit 63070e2 into ME-ICA:master Jan 15, 2021
@tsalo tsalo deleted the fix-duplicate-logs branch January 15, 2021 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Duplicated text in reports
3 participants