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 python 3 issues for tools on test.galaxyproject.org #7818

Closed
20 of 29 tasks
mvdbeek opened this issue Apr 25, 2019 · 9 comments
Closed
20 of 29 tasks

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

mvdbeek opened this issue Apr 25, 2019 · 9 comments
Labels

Comments

@mvdbeek
Copy link
Member

mvdbeek commented Apr 25, 2019

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:

  • toolshed.g2.bx.psu.edu/repos/iuc/hisat2/hisat2/2.1.0
  • toolshed.g2.bx.psu.edu/repos/lparsons/cutadapt/cutadapt/1.16.5
  • testtoolshed.g2.bx.psu.edu/repos/lparsons/cutadapt/cutadapt/1.16.4
  • toolshed.g2.bx.psu.edu/repos/devteam/subtract_query/subtract_query1/0.1

using izip:

should be fixed by #7867

  • toolshed.g2.bx.psu.edu/repos/devteam/bwa/bwa_mem/0.7.17.1
  • toolshed.g2.bx.psu.edu/repos/devteam/bowtie2/bowtie2/2.3.4.2
  • toolshed.g2.bx.psu.edu/repos/devteam/bwa/bwa/0.7.17.4

using iteritems:

Should be fixed by #7867, but can't test because dependencies are not installable via Conda

  • toolshed.g2.bx.psu.edu/repos/devteam/variant_annotator/gatk_variant_annotator/0.0.5
  • toolshed.g2.bx.psu.edu/repos/devteam/variant_filtration/gatk_variant_filtration/0.0.5
  • toolshed.g2.bx.psu.edu/repos/devteam/realigner_target_creator/gatk_realigner_target_creator/0.0.4
  • toolshed.g2.bx.psu.edu/repos/devteam/count_covariates/gatk_count_covariates/0.0.5

token not replaced:

Should be fixed by #7823

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

  • toolshed.g2.bx.psu.edu/repos/iuc/multiqc/multiqc/1.6
  • toolshed.g2.bx.psu.edu/repos/iuc/gemini_gene_wise/gemini_gene_wise/0.20.1
  • toolshed.g2.bx.psu.edu/repos/iuc/gemini_inheritance/gemini_inheritance/0.20.1
  • toolshed.g2.bx.psu.edu/repos/iuc/gemini_load/gemini_load/0.20.1+galaxy1

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

Should be fixed by #7826

  • toolshed.g2.bx.psu.edu/repos/iuc/meme_meme/meme_meme/4.11.0.0

List comprehension scope issue

Should be fixed by #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.

  • toolshed.g2.bx.psu.edu/repos/iuc/circos/circgraph/0.9-RC2

Unprefixed nested variable

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

  • toolshed.g2.bx.psu.edu/repos/devteam/hisat/hisat/1.0.1

unknown reason:

  • toolshed.g2.bx.psu.edu/repos/devteam/kraken2tax/Kraken2Tax/1.1
  • toolshed.g2.bx.psu.edu/repos/devteam/vcfvcfintersect/vcfvcfintersect/0.0.3
  • toolshed.g2.bx.psu.edu/repos/greg/plant_tribes_assembly_post_processor/plant_tribes_assembly_post_processor/1.0.3.0
  • toolshed.g2.bx.psu.edu/repos/greg/plant_tribes_gene_family_classifier/plant_tribes_gene_family_classifier/1.0.3.0
  • toolshed.g2.bx.psu.edu/repos/greg/plant_tribes_gene_family_integrator/plant_tribes_gene_family_integrator/1.0.3.0

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

  • Collect tool ids that use python 2 but don't declare a dependency and inject python 2. We already do something similar for tools that assume they can import from Galaxy
  • Fix the test framework bam-to-sam conversion
  • Figure out what's happening with Error creating a job for these tool inputs - Error executing tool: Requested '__call__' column missing from column def
  • Exclude tests that install a tool data table
  • Check for each tool why it fails to create a job
@mvdbeek mvdbeek added planning area/python3 Specific to Python 3 labels Apr 25, 2019
@mvdbeek
Copy link
Member Author

mvdbeek commented Apr 25, 2019

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

@nsoranzo
Copy link
Member

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
Copy link
Member Author

mvdbeek commented Apr 25, 2019

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
Copy link
Member Author

mvdbeek commented Apr 25, 2019

  • 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
Copy link
Member

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
Copy link
Member Author

mvdbeek commented May 2, 2019

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). #7867 should fix all the (obvious) python 3 errors we've seen on test.galaxyproject.org.

@jxtx
Copy link
Contributor

jxtx commented May 2, 2019

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
Copy link
Member Author

mvdbeek commented May 2, 2019

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

  • we're running on python 3
  • we're using a tool that uses a profile < 19.05 or that does not explicitly set python_template_version to 3
  • compiling the unchanged cheetah module failed

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 #7867 would be more than welcome.

@mvdbeek
Copy link
Member Author

mvdbeek commented Aug 26, 2020

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

@mvdbeek mvdbeek closed this as completed Aug 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants