galaxyproject / galaxy

Data intensive science for everyone.
https://galaxyproject.org
Other
1.37k stars 991 forks source link

Fix python 3 issues for tools on test.galaxyproject.org #7818

Closed mvdbeek closed 4 years ago

mvdbeek commented 5 years ago

To make informed decisions about what needs to be done to have all tools continue working under python 3 we decided to migrate test.galaxyproject.org to python 3 (thanks @natefoo for doing that!) and run the tool tests for all installed tools.

@martenson and me have set up a Jenkins job that can run tool tests against all installed repositories at https://jenkins.galaxyproject.org/view/test_jobs/job/tool%20tests%20against%20test.galaxyproject.org/

The first results are in, you can see a json blob with all results at https://gist.github.com/mvdbeek/fa8b5bde1a7d7c0cdd9cef793714b913 and you can generate an html file using planemo test_reports tool_test_output.json.

The Jenkins job has executed 1021 tool tests, of which 408 tests have failed. For the most part it's just dependency issues and tools that declare no python dependency, but that ship with a python2 only code. Those are easy to take care of, we can collect them and inject a python 2 dependency for those tools

The hard issues are when a tool uses python2-only syntax in Cheetah. These tools will fail to build the command line and in those cases we will not get a exit code or a stderr so I went through the report and I counted 41 tests that fall into this category, distributed over 23 tools:

pass in local test against python 3:

using izip:

should be fixed by https://github.com/galaxyproject/galaxy/pull/7867

using iteritems:

Should be fixed by https://github.com/galaxyproject/galaxy/pull/7867, but can't test because dependencies are not installable via Conda

token not replaced:

Should be fixed by https://github.com/galaxyproject/galaxy/pull/7823

(/private/var/folders/df/6xqpqpcd7h73b6jpx9t6cwhw0000gn/T/tmp3r9q210n/job_working_directory/000/27/tool_script.sh: line 25: @ESCAPE_IDENTIFIER@: command not found)

TypeError: '<' not supported between instances of 'InputValueWrapper' and 'float'

Should be fixed by https://github.com/galaxyproject/galaxy/pull/7826

List comprehension scope issue

Should be fixed by https://github.com/galaxyproject/galaxy/pull/7867

In python 3 list comprehensions have their own scope, so something like

    #for $idx, $data_group in enumerate($sec_tdd.data):
    <param name="sec_tdd|data_${idx}|r0" value="${data_group.r0}" />
    <param name="sec_tdd|data_${idx}|r1" value="${data_group.r1}" />
    <param name="sec_tdd|data_${idx}|orientation" value="${data_group.orientation}" />
    <param name="sec_tdd|data_${idx}|plot_format|plot_format_select" value="${data_group.plot_format.plot_format_select}" />
        <!-- Note, please double check your files -->
    #if str($data_group.plot_format.plot_format_select) == 'histogram':
        #set my_files = ','.join([ "my-test-case/%s-%s.%s" % ($idx, $j, $file.ext) for ($j, $file) in enumerate($data_group.plot_format.data_source)])

won't work (the template generation in Cheetah happens with class scope) and is not fixable by a simple 2to3.

Unprefixed nested variable

I suppose this has worked at some point. This is not a python 3 issue

unknown reason:

Bowtie2 and bwa-mem use izip, which is something we might be able to fix in an automatic fashion using 2to3.

There are still some python 3 bugs that inflate the failing test number. Conversion of bam to sam in the test framework seems to fail with Converting local (test-data) BAM to SAM failed: 'NoneType' object has no attribute 'view' and there's

And there's also some tests that'll never work -- those that install a new tool data table, which we should exclude when collecting tests.

Actionable items

mvdbeek commented 5 years ago

Oh, we just need to install pysam to fix the bam-to-sam conversion issue :)

nsoranzo commented 5 years ago

For the most part it's just dependency issues and tools that declare no python dependency, but that ship with a python2 only code. Those are easy to take care of, we can collect them and inject a python 2 dependency for those tools

It would be more future proof to port them to Python 3 and add a Python 3.x dependency.

mvdbeek commented 5 years ago

Sure, but I think the point is that we want to keep old tools working. We can easily limit the injection with a maximum version.

mvdbeek commented 5 years ago
  • Figure out what's happening with Error creating a job for these tool inputs - Error executing tool: Requested '__call__' column missing from column def

Can't reproduce this locally (this was happening with ncbi_blastdbcmd_info), but this test uses a tool data table, so this may also happen if testing against a remote python 2 Galaxy ... ?

nsoranzo commented 5 years ago

Sure, but I think the point is that we want to keep old tools working. We can easily limit the injection with a maximum version.

Ah, now I see that you meant to patch the tool requirements after it's loaded (or something like that). Good idea!

mvdbeek commented 5 years ago

Finally I think we may get away with not having a python 2 sandbox for evaluating cheetah templates (for now, we may still want to have this for better isolation). https://github.com/galaxyproject/galaxy/pull/7867 should fix all the (obvious) python 3 errors we've seen on test.galaxyproject.org.

jxtx commented 5 years ago

Bowtie2 and bwa-mem use izip, which is something we might be able to fix in an automatic fashion using 2to3.

Should we inject everything from six into the template namespace and see how many that fixes? We could do that conditionally for old tools.

mvdbeek commented 5 years ago

Currently I think running futurize on the compiled Cheetah module code is the best approach. In https://github.com/galaxyproject/galaxy/pull/7867/files we do that only if

If we just do from six import * we probably wouldn't fix itertools.izip that bowtie2 uses, or instances of dict.iter{keys,values,items}(). A review on https://github.com/galaxyproject/galaxy/pull/7867/ would be more than welcome.

mvdbeek commented 4 years ago

I think we've done what we can for making most tools python 3 compatible.

Check for each tool why it fails to create a job

will be reported more clearly with https://github.com/galaxyproject/galaxy/pull/10146/files