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

migrate in emboss_5, fastqc, freebayes, picard from devteam #1109

Merged
merged 19 commits into from
Jan 28, 2017

Conversation

martenson
Copy link
Member

@martenson martenson commented Jan 11, 2017

part of galaxyproject/tools-devteam#406
xref: #1060

ping @galaxyproject/iuc

this is an effort to address part of galaxyproject/tools-devteam#416

@martenson
Copy link
Member Author

thanks to @erasche for helping with figuring out how to treat the missing docs

@martenson
Copy link
Member Author

martenson commented Jan 11, 2017

freebayes fails linting because of <conversion> not being allowed in schema https://docs.galaxyproject.org/en/latest/dev/schema.html

<param name="input_variant_vcf" type="data" format="vcf_bgzip" label="Use variants reported in VCF file as input to the algorithm">
    <conversion name="Tabixized_input" type="tabix" />
</param>

ping @jmchilton - did we miss this tag?

@@ -0,0 +1 @@
../../macros/read_group_macros.xml
Copy link
Member

Choose a reason for hiding this comment

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

@martenson This a symlink to a file that is present only on the tools-devteam repository. Can you add it to the PR?

Copy link
Member

Choose a reason for hiding this comment

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

I've added the file in commit 10954c0 .

@nsoranzo
Copy link
Member

@martenson I've pushed 2 commits to your branch and I plan to do some more work tomorrow, hope that's OK.

@yhoogstrate
Copy link
Member

Same for me (Just one pr)

@nsoranzo
Copy link
Member

I made a massive clean-up of the EMBOSS wrappers in commit cafb44d . More can be done post-merge, e.g.:

  • fix datatype assignment to output datasets
  • change many type="select" params to boolean.

@nsoranzo
Copy link
Member

freebayes fails linting because of <conversion> not being allowed in schema

@martenson @jmchilton I've created galaxyproject/galaxy#3453 to track this.

@nsoranzo
Copy link
Member

@martenson I've added the changes requested in my review of martenson#1 .
@jmchilton If you can update Planemo with the new XSD changes, then the tests should pass.

@@ -0,0 +1,6 @@
<?xml version="1.0"?>
Copy link
Member

@yhoogstrate yhoogstrate Jan 23, 2017

Choose a reason for hiding this comment

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

@nsoranzo Can this be removed?

Edit: nope, emboss is not in bioconda

Copy link
Member

Choose a reason for hiding this comment

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

It is not in Bioconda, but it is in iuc:

$ conda search emboss
Fetching package metadata ...............
emboss                       5.0.0                         0  iuc             
                             6.5.7                         0  bioconda        
                             6.5.7                         1  bioconda        
                             6.5.7                         2  bioconda        

Anyway, I have no strong opinion, maybe we can remove it when we upgrade to a newer EMBOSS version.

@martenson
Copy link
Member Author

martenson commented Jan 26, 2017

All tool tests are failing now, I don't know why.

@nsoranzo
Copy link
Member

The problem was that release_16.10 has not been update to require six 1.10, but now that we have a twill wheel and a new Planemo release (thanks to @jmchilton), the linting passes and only the Java tools fail for same reason. @bgruening ideas?

<regex match="Exception:" />
</stdio>
<command><![CDATA[
python '${__tool_directory__}/rgFastQC.py'
Copy link
Member

Choose a reason for hiding this comment

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

Nicola this looks wired, isn't it?

Copy link
Member

Choose a reason for hiding this comment

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

Why? rgFastQC.py is part of the wrapper.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

I think I've fixed the FastQC issue with commit d38e1e7, let's see.

Copy link
Member

Choose a reason for hiding this comment

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

Oh ok! Good to know. I thought thats the reason for the strange error message. I still think we should change the variable name and keep is consistent with other places.
Thanks NIcola!

Introduced in commit 78ee1ad .
Also consistently use self.opts
@martenson
Copy link
Member Author

fastqc now passes, yay! thanks @nsoranzo

@nsoranzo
Copy link
Member

So FastQC is finally fixed, but all Picard tools still fail because of this error: Exception in thread "main" java.lang.UnsupportedClassVersionError: picard/cmdline/PicardCommandLine : Unsupported major.minor version 52.0

This indicates that Picard tools are run with Java <8 even if the correct java-jdk Bioconda package is installed. Maybe we need to use Galaxy 17.01? @bgruening

@nsoranzo
Copy link
Member

The JAVA_HOME change was not applied during the Travis build, trying to close/reopen.

@nsoranzo nsoranzo closed this Jan 28, 2017
@nsoranzo nsoranzo reopened this Jan 28, 2017
@bgruening
Copy link
Member

Thanks @nsoranzo. Thats seems to fix it.
Ready for the merge?

@nsoranzo
Copy link
Member

Green at last! Thanks all!

@nsoranzo nsoranzo merged commit b583bbe into galaxyproject:master Jan 28, 2017
@martenson
Copy link
Member Author

Splendid. Thanks @nsoranzo @bgruening and @yhoogstrate for help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants