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

reset multiqc between runs #1596

Merged
merged 3 commits into from
Nov 26, 2021
Merged

reset multiqc between runs #1596

merged 3 commits into from
Nov 26, 2021

Conversation

jchorl
Copy link
Contributor

@jchorl jchorl commented Nov 25, 2021

Currently, multiqc leverages global state to store info. If you run it twice, you get weird results - e.g. table columns get duplicated. This moves the init logic into a re-callable function, such that multiqc can reset state between runs.

  • This comment contains a description of changes (with reason)
  • CHANGELOG.md has been updated

I wrote a couple quick scripts to test:

$ cat other.py 
def init():
    global foo
    foo = ["bar"]

$ cat test.py 
import other

other.init()
print(other.foo)
other.foo.append("baz")
print(other.foo)
other.init()
print(other.foo)

$ python3 test.py 
['bar']
['bar', 'baz']
['bar']

@ewels
Copy link
Member

ewels commented Nov 26, 2021

hah, smart! Yes this has been bugging me for a while, one of those things where I was less experienced with Python when I started MultiQC and didn't know better.

I have been vaguely planning to refactor a lot of this code to use objects instead, eg. have a multiqc.Report() object with all of this state information. However that's a bigger job and is one of many things that I've been putting in a v2.0 rewrite in my head. This could be a nice simple workaround until then.. 👍🏻

Copy link
Member

@ewels ewels left a comment

Choose a reason for hiding this comment

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

Not tested locally, but CI is still passing and I think if anything was wrong they would fail.

If you could please just add a small note in the changelog that'd be fab, then I'll merge 👍🏻

@jchorl
Copy link
Contributor Author

jchorl commented Nov 26, 2021

No global state would be nice :)

Updated the changelog.

@ewels ewels merged commit 947eeb9 into MultiQC:master Nov 26, 2021
@ewels
Copy link
Member

ewels commented Nov 26, 2021

The modules already use classes, so it's mostly a report object, a template object(?), a config object and then a MultiQC run object I guess.. If you fancy a challenge 😉

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.

2 participants