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 artic to v.1.6.0 #6693

Open
wants to merge 25 commits into
base: main
Choose a base branch
from
Open

Conversation

ammaraziz
Copy link
Contributor

@ammaraziz ammaraziz commented Jan 24, 2025

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)

This pull request updates the artic wrapper to the latest version (1.6.0), incorporates the clair3 models data manager and also allows the user to manually specify the model from their history. There is one test that is purposefully commented out, it is to test specifying the model from history. The clair3 models are too big and are restricted, therefore they can not be included in the test-data.

Pinging @pvanheus as he is/was working on the clair3 data manager.

There was a minor change to the gupplyplex wrapper to set the min/max as optional parameters. This makes writing workflows easier with optional inputs with a default value.

I have ticked unrestricted checkbox even though it relies on the clair3 tools and the ONT models. The tool itself is MIT licensed.

As always open to any changes!

@pvanheus
Copy link
Contributor

The linting failure is on a URL which is valid. Is there a way to re-run the tests?

@ammaraziz
Copy link
Contributor Author

ammaraziz commented Jan 24, 2025

Linting fails due to 404:

.. ERROR: Error '404 Client Error: Not Found for url: https://labs.primalscheme.com/' accessing https://labs.primalscheme.com/

But that URL works? The URL is mentioned in the help section.

Edit: just saw your message @pvanheus. I've reran the linting again (had to push a small change). Still the same error.

Not sure why its failing on a valid address.

@ammaraziz ammaraziz changed the title Update artic Update artic to v.1.6.0 Jan 25, 2025
@ammaraziz

This comment was marked as outdated.

tools/artic/macros.xml Outdated Show resolved Hide resolved
tools/artic/artic_guppyplex.xml Outdated Show resolved Hide resolved
tools/artic/artic_guppyplex.xml Show resolved Hide resolved
tools/artic/artic_minion.xml Show resolved Hide resolved
tools/artic/artic_minion.xml Show resolved Hide resolved
tools/artic/artic_minion.xml Show resolved Hide resolved
tools/artic/artic_minion.xml Show resolved Hide resolved
tools/artic/artic_minion.xml Show resolved Hide resolved
@ammaraziz
Copy link
Contributor Author

All checks are passing! @bernt-matthias I have incorporated all your comments. Im unsure about the data table integration, if I have gotten it wrong (sorry!) please let me know.

<param name="min_length" type="integer" label="Remove reads shorter than" value="400" help="remove reads less than this number of base pairs" />
<param name="min_quality" type="integer" min="0" value="7"
label="Eliminate reads with a mean base quality score of less than"
<param name="max_length" type="integer" value="700" label="Remove reads longer than" help="remove reads greater than this number of base pairs" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Lab Maximum length would be better IMO.

<param name="min_quality" type="integer" min="0" value="7"
label="Eliminate reads with a mean base quality score of less than"
<param name="max_length" type="integer" value="700" label="Remove reads longer than" help="remove reads greater than this number of base pairs" />
<param name="min_length" type="integer" value="400" label="Remove reads shorter than" help="remove reads less than this number of base pairs" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Same

label="Eliminate reads with a mean base quality score of less than"
<param name="max_length" type="integer" value="700" label="Remove reads longer than" help="remove reads greater than this number of base pairs" />
<param name="min_length" type="integer" value="400" label="Remove reads shorter than" help="remove reads less than this number of base pairs" />
<param name="min_quality"
Copy link
Contributor

Choose a reason for hiding this comment

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

Analogous

@@ -9,4 +9,8 @@
<columns>value, dbkey, name, path</columns>
<file path="${__HERE__}/test-data/all_fasta.loc" />
</table>
<table name="model" comment_char="#">
<columns>value, name, platform, path</columns>
Copy link
Contributor

Choose a reason for hiding this comment

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

Would a version column be useful here? Or often has in other cases.

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.

3 participants