peterjc / galaxy_blast

Galaxy wrappers for NCBI BLAST+ and related BLAST tools.
76 stars 70 forks source link

Update to 2.10.1 #123

Closed abretaud closed 4 years ago

abretaud commented 4 years ago

An attempt at updating to 2.9.0, not sure it will pass the tests, any help welcome! I need a version > 2.8.0 as I'm starting to have some banks in v5 format introduced in 2.8.0. (and not yet 2.10.0 as it's not yet in bioconda)

I haven't made a package for blast 2.9.0 as I think conda should be now enough for most galaxy admins, right? I can remove the tool_dependencies.xml if packages are abandoned with this version

I was also wondering if it would make sense to adopt a more standard tool versioning: like 2.9.0 (and 2.9.0+galaxy1 for future fixes of the xml), what do you think?

I'm also tempted to remove the repository_dependencies.xml as I think blast_datatypes have been included in the galaxy code a while ago?

(just suggestions, maybe there are good reasons to keep things as they are!)

abretaud commented 4 years ago

Green now! I've gone ahead and:

What do you think @peterjc?

I guess https://github.com/galaxyproject/galaxy/pull/9939 should ideally be merged before this one (though there's only a warning during the tests)

abretaud commented 4 years ago

ping @peterjc have you had the time to have a look at the proposed changes?

peterjc commented 4 years ago

Not really, no. Pretty frantic in the run up to the BCC2020 meeting. Might have a chance in the CoFest which follows (will you be there?), but even that might be optimistic...

abretaud commented 4 years ago

Ok, I'll be at the cofest :)

abretaud commented 4 years ago

Just updated to 2.10.1 as the conda package is out now. There are probably a few broken tests now, but no time to fix them right now

abretaud commented 4 years ago

Green now \o/ @peterjc if you're around, would you have time to look at it now?

FredericBGA commented 4 years ago

@abretaud With 2.10 the minimun value for blastn word_size parameter is now 4 (-word_size <Integer, >=4>) https://github.com/peterjc/galaxy_blast/blob/f726ee4fbfef077d2ffe7e1a67eee254babb7841/tools/ncbi_blast_plus/ncbi_blastn_wrapper.xml#L64

https://github.com/peterjc/galaxy_blast/blob/f726ee4fbfef077d2ffe7e1a67eee254babb7841/tools/ncbi_blast_plus/ncbi_macros.xml#L464

As a quick fix I've done here:

<expand macro="input_word_size_blastn" />

and in macro:

    <xml name="input_word_size_blastn">
        <param argument="-word_size" type="integer" min="4" optional="true"
        label="Word size for wordfinder algorithm"
        help="Leave blank for default, otherwise minimum 4" />
    </xml>

I can create a PR for this, or maybe you can add this above?

abretaud commented 4 years ago

Thanks @FredericBGA, is it only for blastn?

FredericBGA commented 4 years ago

Thanks @FredericBGA, is it only for blastn?

yes, sorry it was not obvious. nice, thank you for this PR!

peterjc commented 4 years ago

@FredericBGA a separate PR would be fine on just the blastn word_size change.

FredericBGA commented 4 years ago

@FredericBGA a separate PR would be fine on just the blastn word_size change.

@peterjc It's already done by @abretaud : https://github.com/peterjc/galaxy_blast/pull/123/commits/fe2be94af36710bc63a8d3d5429ee0b7d3b35154 Do you need the PR?

peterjc commented 4 years ago

That’s fine then

On Wed, 9 Sep 2020 at 07:30, Fred notifications@github.com wrote:

@FredericBGA https://github.com/FredericBGA a separate PR would be fine on just the blastn word_size change.

@peterjc https://github.com/peterjc It's already done by @abretaud https://github.com/abretaud : fe2be94 https://github.com/peterjc/galaxy_blast/commit/fe2be94af36710bc63a8d3d5429ee0b7d3b35154

Do you need the PR?

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/peterjc/galaxy_blast/pull/123#issuecomment-689336893, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAPTV7DF5I3EK5UFWXKCSLSE4OHHANCNFSM4MPB5SCQ .

peterjc commented 4 years ago

Closing in favour of #133 (based on this) for v2.9.0, and a followup for v2.10.1 also drawing on this PR.