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

renamed metadata fields #149

Draft
wants to merge 6 commits into
base: dev
Choose a base branch
from
Draft

renamed metadata fields #149

wants to merge 6 commits into from

Conversation

dfornika
Copy link

@dfornika dfornika commented Feb 3, 2025

Renaming metadata field names as requested by @apetkau and @jrober84 (see Issue #148)

PR checklist

  • This comment contains a description of changes (with reason).
  • If you've fixed a bug or added code that should be tested, add tests!
  • If you've added a new tool - have you followed the pipeline conventions in the contribution docs
  • Make sure your code lints (nf-core lint).
  • Ensure the test suite passes (nextflow run . -profile test,docker --outdir <OUTDIR>).
  • Usage Documentation in docs/usage.md is updated.
  • Output Documentation in docs/output.md is updated.
  • CHANGELOG.md is updated.
  • README.md is updated (including new tool citations and authors/contributors).

Copy link

github-actions bot commented Feb 3, 2025

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 @dfornika,

It looks like this pull-request is has been made against the phac-nml/mikrokondo main branch.
The main branch on nf-core 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/mikrokondo 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!

@dfornika dfornika changed the base branch from main to dev February 3, 2025 20:25
Copy link

github-actions bot commented Feb 3, 2025

nf-core pipelines lint overall result: Passed ✅ ⚠️

Posted for pipeline commit c0dd307

+| ✅ 228 tests passed       |+
#| ❔  32 tests were ignored |#
!| ❗  81 tests had warnings |!

❗ Test warnings:

❔ Tests ignored:

  • files_exist - File is ignored: CODE_OF_CONDUCT.md
  • files_exist - File is ignored: assets/nf-core-mikrokondo_logo_light.png
  • files_exist - File is ignored: docs/images/nf-core-mikrokondo_logo_light.png
  • files_exist - File is ignored: docs/images/nf-core-mikrokondo_logo_dark.png
  • files_exist - File is ignored: .github/ISSUE_TEMPLATE/config.yml
  • files_exist - File is ignored: .github/workflows/awstest.yml
  • files_exist - File is ignored: .github/workflows/awsfulltest.yml
  • files_exist - File is ignored: docs/output.md
  • files_exist - File is ignored: docs/README.md
  • files_exist - File is ignored: docs/usage.md
  • nextflow_config - Config variable ignored: manifest.name
  • nextflow_config - Config variable ignored: manifest.homePage
  • nextflow_config - Config variable ignored: params.max_cpus
  • files_unchanged - File does not exist: CODE_OF_CONDUCT.md
  • 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 does not exist: .github/ISSUE_TEMPLATE/config.yml
  • files_unchanged - File ignored due to lint config: .github/ISSUE_TEMPLATE/feature_request.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: .github/workflows/linting.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-mikrokondo_logo_light.png
  • files_unchanged - File does not exist: docs/images/nf-core-mikrokondo_logo_light.png
  • files_unchanged - File does not exist: docs/images/nf-core-mikrokondo_logo_dark.png
  • files_unchanged - File does not exist: docs/README.md
  • files_unchanged - File ignored due to lint config: .gitignore or .prettierignore
  • actions_awstest - 'awstest.yml' workflow not found: /home/runner/work/mikrokondo/mikrokondo/.github/workflows/awstest.yml
  • multiqc_config - multiqc_config

✅ Tests passed:

Run details

  • nf-core/tools version 3.2.0
  • Run at 2025-02-04 00:21:03

@dfornika
Copy link
Author

dfornika commented Feb 3, 2025

In order to add a qc_status_read_length field, should we add a param to define the min_read_length (or min_mean_read_length)?

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 @dfornika, this is awesome work 😄.

For your question on qc_status_read_length, that is a good point about adding a parameter for min_mean_read_length. @mattheww95 is this something that already exists in mikrokondo? As in a check to make sure the mean read length is over a threshold?

"RawReadSummary.R1.mean_sequence_length" : "Mean Sequence Length Forward",
"RawReadSummary.R2.mean_sequence_length" : "Mean Sequence Length Reverse",
Copy link
Member

Choose a reason for hiding this comment

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

Could you align this with the other text (I assume there's a tab here where there should be spaces).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Just following up on this PR. I reached out to @dfornika to mention that I can finish off this PR for him. I just had to wrap something else up.

I did have some questions related to the two new fields, the average read length one will not be too hard to add. But I want to verify some of the requirements for the new fields.

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