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

RGI add parameter to save json files #252

Merged
merged 7 commits into from
Apr 14, 2023
Merged

RGI add parameter to save json files #252

merged 7 commits into from
Apr 14, 2023

Conversation

Darcy220606
Copy link
Contributor

@Darcy220606 Darcy220606 commented Mar 29, 2023 •
edited
Loading

In this PR:

  • CHANGE: RGI outputs by default only .txt and have added the arg_rgi_savejson parameter to give the user the option to save it as part of the RGI output dir if required.
  • REASON: We noticed that RGI outputs HUGE .json files, which are quite bulky and are not required downstream in the pipeline.
  • CLOSES issue Optionally keep RGI json output #245

TODO

  • Usage Documentation in docs/usage.md is updated. NOTYET!!!
  • Output Documentation in docs/output.md is updated. NOTYET!!
  • CHANGELOG.md is updated. NOTYET!!
  • README.md is updated (including new tool citations and authors/contributors).

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- [ ] If necessary, also make a PR on the nf-core/funcscan branch on the nf-core/test-datasets repository.
  • Make sure your code lints (nf-core lint).
  • Ensure the test suite passes (nextflow run . -profile test,docker --outdir <OUTDIR>).

jfy133
jfy133 reviewed Mar 29, 2023
View reviewed changes
Copy link
Member

@jfy133 jfy133 left a comment

Choose a reason for hiding this comment

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

CHANGEOG update?

@Darcy220606
Copy link
Contributor Author

Have to catch the train, will do the documentation part later when im back :)

@github-actions
Copy link

github-actions bot commented Mar 29, 2023 •
edited
Loading

nf-core lint overall result: Passed ✅ ⚠️

Posted for pipeline commit 7d1b977

+| ✅ 157 tests passed       |+
!| ❗   1 tests had warnings |!

❗ Test warnings:

  • pipeline_todos - TODO string in methods_description_template.yml: #Update the HTML below to your prefered methods description, e.g. add publication citation for this pipeline

✅ Tests passed:

Run details

  • nf-core/tools version 2.7.2
  • Run at 2023-04-14 13:40:23

louperelo
louperelo approved these changes Mar 30, 2023
View reviewed changes
Copy link
Contributor

@louperelo louperelo left a comment

Choose a reason for hiding this comment

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

LGTM

jasmezz
jasmezz requested changes Apr 3, 2023
View reviewed changes
jasmezz
jasmezz requested changes Apr 3, 2023
View reviewed changes
@jfy133 jfy133 linked an issue Apr 4, 2023 that may be closed by this pull request
Optionally keep RGI json output #245
Closed
@jfy133 jfy133 marked this pull request as draft April 4, 2023 10:52
@Darcy220606 Darcy220606 marked this pull request as ready for review April 13, 2023 16:37
@Darcy220606 Darcy220606 requested a review from jasmezz April 13, 2023 16:48
jasmezz
jasmezz approved these changes Apr 14, 2023
View reviewed changes
Copy link
Collaborator

@jasmezz jasmezz left a comment

Choose a reason for hiding this comment

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

Thanks for the updates!
Why are bakta tests failing though? Also for PR #242 it looks like. Could you check if we can fix them here or in another PR (run local test and check for errors)?

@Darcy220606 Darcy220606 merged commit 91c9f8c into nf-core:dev Apr 14, 2023
This was referenced Apr 20, 2023
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jfy133 jfy133 jfy133 left review comments

@jasmezz jasmezz jasmezz approved these changes

@louperelo louperelo louperelo 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.

Optionally keep RGI json output
4 participants
@Darcy220606 @jfy133 @louperelo @jasmezz