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

Chaining proteinfold #176

Merged
merged 39 commits into from
Nov 27, 2024
Merged

Chaining proteinfold #176

merged 39 commits into from
Nov 27, 2024

Conversation

luisas
Copy link
Collaborator

@luisas luisas commented Nov 25, 2024

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/multiplesequencealign 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>).
  • Check for unexpected warnings in debug mode (nextflow run . -profile debug,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).

@github-actions GitHub Actions
Copy link

This PR is against the master 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 @luisas,

It looks like this pull-request is has been made against the luisas/multiplesequencealign master branch.
The master branch on nf-core repositories should always contain code from the latest release.
Because of this, PRs to master are only allowed if they come from the luisas/multiplesequencealign 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!

@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 25, 2024 •
edited
Loading

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

Posted for pipeline commit ccb04b0

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

❗ Test warnings:

  • schema_params - Schema param genome not found from nextflow config
  • schema_params - Schema param fasta not found from nextflow config

❔ Tests ignored:

✅ Tests passed:

Run details

  • nf-core/tools version 3.0.2
  • Run at 2024-11-27 15:59:21

@luisas luisas changed the base branch from master to dev November 25, 2024 15:55
JoseEspinosa
JoseEspinosa requested changes Nov 27, 2024
View reviewed changes
Copy link
Member

@JoseEspinosa JoseEspinosa left a comment

Choose a reason for hiding this comment

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

As we discussed I won't use the word dependencies but something such as optional_data since the work dependencies seems to suggest that this data is needed to run the pipeline. Otherwise, just small suggestions

docs/usage/chaining_with_proteinfold.md Outdated Show resolved Hide resolved
docs/usage/chaining_with_proteinfold.md Outdated Show resolved Hide resolved
docs/usage/chaining_with_proteinfold.md Outdated
```

> [!NOTE]
> The one imporant parameter NOT to forget in proteinfold for the chaining is `--split_fasta`. This will allow to use a multifasta file as input for monomer predictions, needed by the MSA pipeline. Also, currently the changes needed for the chaining are only present in the dev branch of proteinfold, so also do not forget `-r dev`. The rest of the proteinfold parameters can and should be tuned according to your preferences for your proteinfold run. Please refer to the proteinfold documentation for this.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe add a warning or a note saying that this is still a experimental feature on proteinfold that will be release in the near future?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yess

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

added now

docs/usage/chaining_with_proteinfold.md Outdated Show resolved Hide resolved
nextflow.config Outdated
@@ -13,6 +13,7 @@ params {
input = null
tools = null
templates_suffix = ".pdb"
dependencies_dir = null
Copy link
Member

Choose a reason for hiding this comment

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

I will be more explicit with the naming, to me dependencies could be many things

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Okii changed!

workflows/multiplesequencealign.nf Outdated Show resolved Hide resolved
workflows/multiplesequencealign.nf Outdated Show resolved Hide resolved
workflows/multiplesequencealign.nf Outdated Show resolved Hide resolved
workflows/multiplesequencealign.nf Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
luisas and others added 11 commits November 27, 2024 12:32
@luisas @JoseEspinosa
Update README.md
386d973 
Co-authored-by: Jose Espinosa-Carrasco <[email protected]>
@luisas @JoseEspinosa
Update docs/usage/chaining_with_proteinfold.md
e068acc 
Co-authored-by: Jose Espinosa-Carrasco <[email protected]>
@luisas @JoseEspinosa
Update docs/usage/chaining_with_proteinfold.md
9928909 
Co-authored-by: Jose Espinosa-Carrasco <[email protected]>
@luisas @JoseEspinosa
Update workflows/multiplesequencealign.nf
052321d 
Co-authored-by: Jose Espinosa-Carrasco <[email protected]>
@luisas @JoseEspinosa
Update workflows/multiplesequencealign.nf
121e75f 
Co-authored-by: Jose Espinosa-Carrasco <[email protected]>
@luisas @JoseEspinosa
Update docs/usage/chaining_with_proteinfold.md
21037be 
Co-authored-by: Jose Espinosa-Carrasco <[email protected]>
@luisas @JoseEspinosa
Update workflows/multiplesequencealign.nf
54a0030 
Co-authored-by: Jose Espinosa-Carrasco <[email protected]>
@luisas @JoseEspinosa
Update workflows/multiplesequencealign.nf
802741a 
Co-authored-by: Jose Espinosa-Carrasco <[email protected]>
@luisas @JoseEspinosa
Update workflows/multiplesequencealign.nf
54e2f50 
Co-authored-by: Jose Espinosa-Carrasco <[email protected]>
@luisas
up
66bf019
@luisas
update docs
91026c6
This was referenced Nov 27, 2024
Closed
Closed
JoseEspinosa
JoseEspinosa approved these changes Nov 27, 2024
View reviewed changes
Copy link
Member

@JoseEspinosa JoseEspinosa left a comment

Choose a reason for hiding this comment

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

Just a small update (proteinfold version) but otherwise LGTM, nice job!
🚀

docs/usage/chaining_with_proteinfold.md Outdated Show resolved Hide resolved
@luisas luisas merged commit 2da7cf0 into nf-core:dev Nov 27, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JoseEspinosa JoseEspinosa JoseEspinosa 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
@luisas @JoseEspinosa