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

Update output docs for collapsed abundance tables #672

Merged
merged 3 commits into from
Dec 7, 2023
Merged

Update output docs for collapsed abundance tables #672

merged 3 commits into from
Dec 7, 2023

Conversation

danclaytondev
Copy link
Contributor

Hi,

This PR is a minor output documentation update.

Recently I was confused why the qiime output relative abundance tsvs only went to level 6, not level 7 when the reference database we are using includes level 7. In the parameter schema, it shows that the level is controlled by --tax_agglom_max which defaults to 6, so hopefully the output docs better reflect the situation with this change.

I agree that level 6 is a sensible default, I am not proposing to change that, just the output docs! :)

To verify the changed documentation, I believe this is the code that collapses the taxa for absolute abundance counts, and relative abundance counts.

@d4straub Apologies, reopened the PR after fixing the committer details.

@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 7, 2023 •
edited
Loading

nf-core lint overall result: Passed ✅ ⚠️

Posted for pipeline commit feacdd5

+| ✅ 154 tests passed       |+
#| ❔   3 tests were ignored |#
!| ❗   2 tests had warnings |!

❗ Test warnings:

  • readme - README did not have a Nextflow minimum version badge.
  • schema_lint - Parameter input is not defined in the correct subschema (input_output_options)

❔ Tests ignored:

✅ Tests passed:

Run details

  • nf-core/tools version 2.10
  • Run at 2023-12-07 12:43:24

@d4straub
Copy link
Collaborator

d4straub commented Dec 7, 2023

Thanks!
Looks good, but please add a row to the CHANGELOG.md, probably under headline Fixed such as - [#672](https://github.com/nf-core/ampliseq/pull/672) - Update output docs for collapsed abundance tables

d4straub
d4straub approved these changes Dec 7, 2023
View reviewed changes
Copy link
Collaborator

@d4straub d4straub left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good to me!
Do you want to merge it (after all tests below pass)?

edit: I assume nf-core linting / Prettier below fails because there is no blank row between the headline and your addition. For more on the topic see https://nf-co.re/docs/contributing/code_formatting

@danclaytondev
Copy link
Contributor Author

Yep - I am happy to merge! :) (I don't have write access)

@d4straub
Copy link
Collaborator

d4straub commented Dec 7, 2023

I think for write access you need to be part of the github nf-core community, here is how to join: https://nf-co.re/join#github

@danclaytondev danclaytondev merged commit 4e10651 into nf-core:dev Dec 7, 2023
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@d4straub d4straub d4straub 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.
2 participants
@danclaytondev @d4straub