MultiQC / MultiQC

Aggregate results from bioinformatics analyses across many samples into a single report.
http://multiqc.info
GNU General Public License v3.0
1.22k stars 601 forks source link

generated entry point is "wrong" #1261

Closed bernt-matthias closed 3 years ago

bernt-matthias commented 4 years ago

Description of bug:

The executable generated by pip looks like this

#!/tmp/mqcenv/bin/python3
# -*- coding: utf-8 -*-
import re
import sys
from multiqc.__main__ import multiqc
if __name__ == '__main__':
    sys.argv[0] = re.sub(r'(-script\.pyw|\.exe)?$', '', sys.argv[0])
    sys.exit(multiqc())

Here multiqc is a module, hence calling multiqc() on the last line would produce

  File "/tmp/bioconda_cli/multiqc=1.9/bin/multiqc", line 11, in <module>
    sys.exit(multiqc())
TypeError: 'module' object is not callable

For "normal" use of multiqc this is not a problem, since it seems this line is never executed since sys.exit() is always called from inside the code that is executed upon importing from multiqc.__main__ import multiqc.

I stumbled over this because I chose multiqc as an example project for argparse2tool https://github.com/hexylena/argparse2tool for click -> cwl generation. Here sys.exit() is not called .. hence the report.

ewels commented 4 years ago

Great catch, thanks! I guess this was left over when I refactored the main MultiQC function, but as it's never executed as you say - I never noticed.

I will look into fixing / removing / refactoring the code above it when I'm back from holiday.

ewels commented 3 years ago

Hi @bernt-matthias,

I just came back to take a look at this and am a little lost so I'm going to need a few more pointers sorry. When I first read your issue I assumed that the main MultiQC function was called multiqc() which is how it was ending up in that autogenerated launch script. But it's not (at least, not for a while now) - it's called multiqc.run_multiqc(). In fact there are no functions in the MultiQC codebase called multiqc() that I can find.

I don't really understand how these autogenerated launch scripts are built to be honest. Mine looks a little different (pip version 21.0.1) with sys.exit(load_entry_point('multiqc', 'console_scripts', 'multiqc')()) instead.

Basically I'm not sure what it is that you want me to change 😀

Phil

bernt-matthias commented 3 years ago

Hmm. Also a bit mysterious to me. I just pip installed it again and got the same code.

I guess the point is that if I execute from multiqc.__main__ import multiqc in a python shell this already starts multiqc and exits immediately because of missing command line arguments. So the if __name__ == '__main__': is never called.

But I can not recall exactly how I ended up in a case where the line was executed. My guess is that in the argparse2tool project the click library is replaced and this replacement does not exit even if no arguments are given.

I do not understand the import from multiqc.__main__ import multiqc .. since there is no mutiqc function in multiqc.__main__, . but only an import of multiqc.

ewels commented 3 years ago

Yeah, exactly - __main__.py imports the multiqc package:

https://github.com/ewels/MultiQC/blob/c822cc29ad4754d3c2b3afee3674ade25c570b56/multiqc/__main__.py#L14

So that should be no different to just import multiqc I think..? But if I try both of these then I see the same behaviour as you:

$ python
Python 3.7.10 | packaged by conda-forge | (default, Feb 19 2021, 15:59:12)
[Clang 11.0.1 ] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from multiqc.__main__ import multiqc
Usage: multiqc [OPTIONS] <analysis directory>

Error: Missing argument '<analysis directory>'.

This is MultiQC v1.11.dev0 (c822cc2)

For more help, run 'multiqc --help' or visit http://multiqc.info
$ python
Python 3.7.10 | packaged by conda-forge | (default, Feb 19 2021, 15:59:12)
[Clang 11.0.1 ] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import multiqc
>>>

I guess that there is maybe some magic in the click library that I don't understand? Or some kind of flow logic in the imports that I am misunderstanding.

Anyway, as I'm not really sure what I should / can change here I think I will close the issue if that's ok. Feel free to comment / reopen if there is something I can do to help 👍🏻