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

Update minimap2.xml #6707

Merged
merged 8 commits into from
Feb 4, 2025
Merged

Update minimap2.xml #6707

merged 8 commits into from
Feb 4, 2025

Conversation

gdroc
Copy link
Contributor

@gdroc gdroc commented Jan 29, 2025

Moved the --eqx optional parameter from samtools to minimap2

fixes: #6698

FOR CONTRIBUTOR:

  • I have read the CONTRIBUTING.md document and this tool is appropriate for the tools-iuc repo.
  • License permits unrestricted use (educational + commercial)
  • This PR adds a new tool or tool collection
  • This PR updates an existing tool or tool collection
  • This PR does something else (explain below)

gdroc and others added 2 commits January 29, 2025 14:56
Moved the --eqx optional parameter from samtools to minimap2
Copy link
Member

@bgruening bgruening left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks @gdroc

Copy link
Contributor Author

@gdroc gdroc left a comment

Choose a reason for hiding this comment

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

increment version

@@ -32,24 +40,47 @@
<option value="-q">Sort output by query IDs and coordinates (-q)</option>
<option value="-r">Sort output by reference IDs and coordinates (-r)</option>
</param>
<conditional name="format">
Copy link
Member

Choose a reason for hiding this comment

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

Why do you need a conditional here? Can you not just use the boolean param.

Copy link
Member

Choose a reason for hiding this comment

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

What do you think about this comment @gdroc ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just updated the code as you advised me.

Copy link
Member

@bgruening bgruening left a comment

Choose a reason for hiding this comment

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

Thanks for working on the second tool. It would be better to use a separate PR for that. I can not merge the minimap2 changes now :)

@gdroc
Copy link
Contributor Author

gdroc commented Feb 3, 2025

Sorry for this mistake, what do you advise me to do? Close this PR ?

@bgruening
Copy link
Member

We can also work further on this PR I don't mind. Can you comment maybe on my comment for the conditional?

<actions>
<action name="column_names" type="metadata" default="[S1], [E1], [S2], [E2], [LEN 1], [LEN 2], [% IDY], [LEN R], [LEN Q], [COV R], [COV Q], [REF TAG], [QUERY TAG]" />
</actions>
</data>
<data name="output_extend" format="tabular" from_work_dir="show-coords_extend.txt" label="${tool.name} on ${on_string} extend format)" >
<filter>direction is True</filter>
Copy link
Member

Choose a reason for hiding this comment

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

Maybe add also column_names here as in "output"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@bgruening bgruening merged commit 980f747 into galaxyproject:main Feb 4, 2025
14 checks passed
@bgruening
Copy link
Member

Thanks a lot!

@gdroc gdroc mentioned this pull request Feb 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Minimap2
2 participants