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

[TheiaProk suite] Patch fix: change type of kraken2_report to be string in taxon_table task #297

Merged
merged 3 commits into from
Jan 12, 2024

Conversation

cimendes
Copy link
Member

@cimendes cimendes commented Jan 12, 2024

Closes #298

🛠️ Changes Being Made

This PR addresses an issue found during validation where the type for the krakrn2 report output was not being passed correctly to the export taxon table task.

This has been patched.

Impacted Workflows/Tasks

TheiaProk suite of workflows:

  • TheiaProk_Illumina_PE
  • TheiaProk_Illumina_SE

🧠 Context and Rationale

export taxon tables task was expecting a File input for the kraken_report but due to it being an optional output, it is being handled as a String by the Theiaprok workflow (SE and PE).

📋 Workflow/Task Steps

Inputs

N/A

Outputs

N/A

Impacted Outputs

🧪 Testing

Locally

N/A

Terra

Set of 20 bacterial genomes with taxon tables task enabled, as well as kraken2 optional module (one expected failure due to SRA lite file format issue): https://app.terra.bio/#workspaces/theiagen-validations/Theiagen_Mendes_Sandbox/job_history/805a3537-a339-4b05-99bd-8ec4c1b18a32

Scenarios for Reviewer to Test

🔬 Quality checks

Pull Request (PR) checklist:

  • Include a description of what is in this pull request in this message.
  • The workflow/task has been tested locally and on Terra
  • The CI/CD has been adjusted and tests are passing
  • Everything follows the style guide

@cimendes cimendes requested a review from sage-wright January 12, 2024 17:29
@cimendes cimendes marked this pull request as ready for review January 12, 2024 17:42
@cimendes cimendes requested a review from jrotieno January 12, 2024 17:43
Copy link
Member

@sage-wright sage-wright left a comment

Choose a reason for hiding this comment

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

will merge once CI is working

Copy link
Contributor

@kapsakcj kapsakcj left a comment

Choose a reason for hiding this comment

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

CI updated and fixed. thanks for this change.

@kapsakcj kapsakcj merged commit efe6e46 into main Jan 12, 2024
7 checks passed
@kapsakcj kapsakcj deleted the im-theiaprok-kraken-fix branch January 12, 2024 18:58
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.

Kraken2 report output variable type breaks export taxon table task
3 participants