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

fix: Feature/update cutadapt wrapper #67

Merged
merged 5 commits into from
Feb 6, 2023
Merged

fix: Feature/update cutadapt wrapper #67

merged 5 commits into from
Feb 6, 2023

Conversation

Austin-s-h
Copy link
Contributor

I encountered an error in this trimming rule due to

  File "~/mambaforge/envs/snakemake/lib/python3.10/site-packages/snakemake/__init__.py", line 22, in <module>
    from snakemake.target_jobs import parse_target_jobs_cli_args
  File "~/mambaforge/envs/snakemake/lib/python3.10/site-packages/snakemake/target_jobs.py", line 4, in <module>
    from snakemake.common import parse_key_value_arg
  File "~/mambaforge/envs/snakemake/lib/python3.10/site-packages/snakemake/common/__init__.py", line 284, in <module>
    @contextlib.asynccontextmanager
AttributeError: module 'contextlib' has no attribute 'asynccontextmanager'

Tried running under Python 3.10 and 3.7, still encountered the same issue, but eventually tracked the rabbit hole down to this.

Updating the wrapper for cutadapt slightly changes the syntax, but no modification to the config.yaml file is needed. Additionally, (optionally) I added multithreading, as cutadapt scales quite well.

@Austin-s-h Austin-s-h changed the title Feature/update cutadapt wrapper fix: Feature/update cutadapt wrapper Feb 5, 2023
@Austin-s-h Austin-s-h closed this Feb 5, 2023
@Austin-s-h Austin-s-h reopened this Feb 6, 2023
dlaehnemann
dlaehnemann requested changes Feb 6, 2023
View reviewed changes
Copy link
Collaborator

@dlaehnemann dlaehnemann left a comment

Choose a reason for hiding this comment

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

Many thanks for catching and debugging this! Just one little thing that I would switch back.

config/config.yaml Outdated Show resolved Hide resolved
workflow/rules/trim.smk Show resolved Hide resolved
@Austin-s-h @dlaehnemann
Update config/config.yaml
6cc1c64 
Co-authored-by: David Laehnemann <david.laehnemann@hhu.de>
@dlaehnemann
Copy link
Collaborator

Ah, I think it is failing because this string here is empty (and we have a single end test case unit):
https://github.com/sansterbioanalytics/rna-seq-kallisto-sleuth/blob/6cc1c649c668a0d63ed33668f210d89c72db208f/.test/config/config.yaml#L107

Maybe just replicate the settings string from cutadapt-pe in the .test/config/config.yaml and simply omit the capital letter adapter specs (-A and -G, they are for the reverse reads in the paired end setup):
https://github.com/sansterbioanalytics/rna-seq-kallisto-sleuth/blob/6cc1c649c668a0d63ed33668f210d89c72db208f/.test/config/config.yaml#L118

@Austin-s-h
Copy link
Contributor Author

Ah, I think it is failing because this string here is empty (and we have a single end test case unit): https://github.com/sansterbioanalytics/rna-seq-kallisto-sleuth/blob/6cc1c649c668a0d63ed33668f210d89c72db208f/.test/config/config.yaml#L107

Maybe just replicate the settings string from cutadapt-pe in the .test/config/config.yaml and simply omit the capital letter adapter specs (-A and -G, they are for the reverse reads in the paired end setup): https://github.com/sansterbioanalytics/rna-seq-kallisto-sleuth/blob/6cc1c649c668a0d63ed33668f210d89c72db208f/.test/config/config.yaml#L118

Should have addressed this in my latest :)

dlaehnemann
dlaehnemann reviewed Feb 6, 2023
View reviewed changes
.travis.yml
@@ -20,4 +20,4 @@ install:
script:
- source activate snakemake
# run the workflow
- snakemake --use-conda --directory .test || (for f in .test/logs/*; do echo $f; cat $f; done; for f in .test/logs/*/*; do echo $f; cat $f; done; exit 1)
- snakemake --cores all --use-conda --directory .test || (for f in .test/logs/*; do echo $f; cat $f; done; for f in .test/logs/*/*; do echo $f; cat $f; done; exit 1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

BTW, I think this .travis.yml file is an artifact from a time, when CI tests were run via Travis CI. But we have long moved to GitHub Actions, so all testing harness definitions are found in .github/workflows/. I guess we can simply remove this file here in the near future.

@dlaehnemann dlaehnemann merged commit 29d7967 into snakemake-workflows:main Feb 6, 2023
@github-actions github-actions bot mentioned this pull request Feb 6, 2023
Merged
@Austin-s-h Austin-s-h deleted the feature/update-cutadapt-wrapper branch February 6, 2023 18:57
Austin-s-h added a commit to sansterbioanalytics/rna-seq-kallisto-sleuth that referenced this pull request Feb 6, 2023
@Austin-s-h
Merge pull request #4 from snakemake-workflows/main
b1c89b3 
fix: Feature/update cutadapt wrapper (snakemake-workflows#67)
johanneskoester pushed a commit that referenced this pull request Feb 10, 2023
@github-actions
chore(main): release 2.4.3 (#69)
08082ce 
🤖 I have created a release *beep* *boop*
---


##
[2.4.3](v2.4.2...v2.4.3)
(2023-02-06)


### Bug Fixes

* Feature/update cutadapt wrapper
([#67](#67))
([29d7967](29d7967))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This was referenced Feb 10, 2023
Open
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dlaehnemann dlaehnemann dlaehnemann 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
@Austin-s-h @dlaehnemann