Workflow #4
Merged
Workflow #4
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 2 commits
April 2, 2024 16:09
Implemented the initial workflow, it is likely that many changes will need to be made. However I am focusing on implementing working unit-tests for the time being
I added some unit tests to the pipeline however these tests are incomplete, and show some current work that needs to be done in terms of altering the output paths as they are quite messy currently. Additionally more tests with the existing data can be added as it woul really improve the robustness of the pipeline
|
added 10 commits
April 2, 2024 16:27
Its likely that the binaries for the individual tools were not being found as the nf-test.config file was not configured to use docker.
|
Linting tests are failing as I did not update the schema. I wanted prioritize getting the main "bones" of the pipeline in with test data so that critical feedback can be provided on the structure of the pipeline itself to make sure it fits what is expected. I did not set up the irida-next.conf either yet as well, as I think the output paths will change and we may want to discuss what is stored in terms of data being parsed as metadata for IRIDA Next. |
emarinier
reviewed
Apr 3, 2024
emarinier
reviewed
Apr 3, 2024
emarinier
requested changes
Apr 3, 2024
There was a problem hiding this comment.
I think the changes here look good overall. I have some general comments:
- It might be worth adding failure / error testing.
- Generally, I'd advise against merging any TODOs into a dev / main branch, and instead do the TODO or make an issue on GitHub to do it later.
- I assume we're going forward with default parameters all listed in the
nextflow.config?
|
Oh, also getting the tests to pass. |
|
Good news all tests passed! I am not proud of it, but to resolve the TODO I changed it to a point for discussion. As for the defualt parameters, I dont know how most of the parameters to be passed to the tools are meant to be used so I just left them there for now. I can add failure testing as well! I just thought it would be better to leave that as a seperate PR, I just wanted to get the initial skeleton of the tests implemented and full disclosure I was going to bug you about those afterwards... |
emarinier
approved these changes
Apr 4, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Created an initial workflow and added basic tests, with some data. Linting will likely fail as I have not updated the
nextflow_schema.jsonyet.