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.

Sign up for GitHub

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

Jump to bottom

Add module to create the expected_clusters file for gas-call #11

Merged
merged 14 commits into from
Jun 10, 2024
Merged

Add module to create the expected_clusters file for gas-call #11

merged 14 commits into from
Jun 10, 2024

Conversation

kylacochrane
Copy link
Contributor

@kylacochrane kylacochrane commented May 27, 2024 •
edited
Loading

This PR eliminates the necessity of the --ref_clusters input parameter file in gasnomenclature, which is currently used to fulfill the --rclusters parameter for the gas_call module. Instead, it introduces a module within the pipeline that utilizes the provided metadata address to dynamically generate the required --rclusters information in the correct format for gas_call.

The CLUSTER_FILE module now generates a tab-separated values (TSV) file called expected_clusters.txt. This file organizes sample addresses into columns based on the maximum number of hierarchical levels found among all reference samples.

NOTE: Previously the expected_clusters.txt file that was provided externally used the incorrect address for sample3. Due to the change, the tests/data/call/expected_results.txt file was updated.

PR checklist

  • This comment contains a description of changes (with reason).
  • Make sure your code lints (nf-core lint).
  • Ensure the test suite passes (nextflow run . -profile test,docker --outdir <OUTDIR>).

@github-actions GitHub Actions
Copy link

This PR is against the main branch ❌

  • Do not close this PR
  • Click Edit and change the base to dev
  • This CI test will remain failed until you push a new commit

Hi @kylacochrane,

It looks like this pull-request is has been made against the phac-nml/gasnomenclature main branch.
The main branch on phac-nml repositories should always contain code from the latest release.
Because of this, PRs to main are only allowed if they come from the phac-nml/gasnomenclature dev branch.

You do not need to close this PR, you can change the target branch to dev by clicking the "Edit" button at the top of this page.
Note that even after this, the test will continue to show as failing until you push a new commit.

Thanks again for your contribution!

@kylacochrane kylacochrane changed the base branch from main to dev May 27, 2024 19:37
@github-actions GitHub Actions
Copy link

github-actions bot commented May 27, 2024 •
edited
Loading

nf-core lint overall result: Passed ✅ ⚠️

Posted for pipeline commit 6691dea

+| ✅ 149 tests passed       |+
#| ❔  22 tests were ignored |#
!| ❗  14 tests had warnings |!

❗ Test warnings:

  • schema_lint - Schema $id should be https://raw.githubusercontent.com/phac-nml/gasnomenclature/master/nextflow_schema.json
    Found https://raw.githubusercontent.com/phac-nml/gasnomenclature/main/nextflow_schema.json
  • schema_description - No description provided in schema for parameter: gm_thresholds
  • schema_description - No description provided in schema for parameter: ref_clusters
  • schema_description - No description provided in schema for parameter: pd_outfmt
  • schema_description - No description provided in schema for parameter: pd_distm
  • schema_description - No description provided in schema for parameter: pd_missing_threshold
  • schema_description - No description provided in schema for parameter: pd_sample_quality_threshold
  • schema_description - No description provided in schema for parameter: pd_match_threshold
  • schema_description - No description provided in schema for parameter: pd_file_type
  • schema_description - No description provided in schema for parameter: pd_mapping_file
  • schema_description - No description provided in schema for parameter: pd_force
  • schema_description - No description provided in schema for parameter: pd_skip
  • schema_description - No description provided in schema for parameter: pd_columns
  • schema_description - No description provided in schema for parameter: pd_count_missing

❔ Tests ignored:

  • files_exist - File is ignored: assets/nf-core-gasnomenclature_logo_light.png
  • files_exist - File is ignored: docs/images/nf-core-gasnomenclature_logo_dark.png
  • files_exist - File is ignored: docs/images/nf-core-gasnomenclature_logo_light.png
  • files_exist - File is ignored: .github/workflows/awstest.yml
  • files_exist - File is ignored: .github/workflows/awsfulltest.yml
  • nextflow_config - Config variable ignored: manifest.name
  • nextflow_config - Config variable ignored: manifest.homePage
  • files_unchanged - File ignored due to lint config: LICENSE or LICENSE.md or LICENCE or LICENCE.md
  • files_unchanged - File ignored due to lint config: .github/CONTRIBUTING.md
  • files_unchanged - File ignored due to lint config: .github/ISSUE_TEMPLATE/bug_report.yml
  • files_unchanged - File ignored due to lint config: .github/PULL_REQUEST_TEMPLATE.md
  • files_unchanged - File ignored due to lint config: .github/workflows/branch.yml
  • files_unchanged - File ignored due to lint config: assets/email_template.html
  • files_unchanged - File ignored due to lint config: assets/email_template.txt
  • files_unchanged - File ignored due to lint config: assets/sendmail_template.txt
  • files_unchanged - File does not exist: assets/nf-core-gasnomenclature_logo_light.png
  • files_unchanged - File does not exist: docs/images/nf-core-gasnomenclature_logo_light.png
  • files_unchanged - File does not exist: docs/images/nf-core-gasnomenclature_logo_dark.png
  • files_unchanged - File ignored due to lint config: docs/README.md
  • actions_awstest - 'awstest.yml' workflow not found: /home/runner/work/gasnomenclature/gasnomenclature/.github/workflows/awstest.yml
  • actions_awsfulltest - actions_awsfulltest
  • pipeline_name_conventions - pipeline_name_conventions

✅ Tests passed:

Run details

  • nf-core/tools version 2.14.1
  • Run at 2024-06-07 18:28:08

@kylacochrane kylacochrane marked this pull request as ready for review May 27, 2024 21:28
@kylacochrane kylacochrane requested a review from apetkau May 27, 2024 21:28
apetkau
apetkau requested changes May 28, 2024
View reviewed changes
Copy link
Member

@apetkau apetkau left a comment

Choose a reason for hiding this comment

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

Thanks so much for this @kylacochrane . This is amazing 😄

In addition to the comments I gave, I'm wondering if you could add process test cases for the CLUSTER_FILE process (https://www.nf-test.com/docs/testcases/nextflow_process/) so that we can more easily test error conditions (such as different number of levels in some addresses)?

modules/local/cluster_file/main.nf Outdated Show resolved Hide resolved
modules/local/cluster_file/main.nf Outdated Show resolved Hide resolved
@kylacochrane
Copy link
Contributor Author

kylacochrane commented May 30, 2024 •
edited
Loading

I have added the CLUSTER_FILE process test here: bd0cd1a

Currently, it only includes a test case where one input sample address is at a different level. Other test cases have been addressed by the gm_delimiter pattern in the nextflow_schema. These test cases will be provided in a separate PR.

@kylacochrane kylacochrane requested review from apetkau and emarinier May 30, 2024 18:39
emarinier
emarinier requested changes Jun 6, 2024
View reviewed changes
Copy link
Member

@emarinier emarinier left a comment

Choose a reason for hiding this comment

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

Looks good, possibly just one very small thing.

modules/local/cluster_file/main.nf Outdated Show resolved Hide resolved
nextflow.config Outdated Show resolved Hide resolved
@emarinier emarinier mentioned this pull request Jun 6, 2024
Merged
6 tasks
apetkau
apetkau requested changes Jun 6, 2024
View reviewed changes
Copy link
Member

@apetkau apetkau left a comment

Choose a reason for hiding this comment

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

Thanks so much @kylacochrane for addressing my comments 😄 . I have a few additional comments.

tests/modules/cluster_file/main.nf.test Show resolved Hide resolved
modules/local/cluster_file/main.nf Outdated Show resolved Hide resolved
@kylacochrane kylacochrane requested review from apetkau and emarinier June 7, 2024 19:30
apetkau
apetkau approved these changes Jun 7, 2024
View reviewed changes
Copy link
Member

@apetkau apetkau left a comment

Choose a reason for hiding this comment

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

Amazing. Thanks so much Kyla for your great work 😄

@kylacochrane kylacochrane merged commit 4acdb8c into dev Jun 10, 2024
4 checks passed
@kylacochrane kylacochrane deleted the expected-clusters branch June 14, 2024 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@emarinier emarinier emarinier approved these changes

@apetkau apetkau apetkau approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kylacochrane @apetkau @emarinier