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

Busco subworkflow #28

Closed
wants to merge 143 commits into from
Closed

Busco subworkflow #28

wants to merge 143 commits into from

Conversation

alxndrdiaz
Copy link
Collaborator

@alxndrdiaz alxndrdiaz commented Sep 5, 2022

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/blobtoolkit 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>).
  • 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).

This PR is part of the Google Summer of Code 2022 project: Conversion of the BlobToolKit pipeline to Nextflow, GitHub repository: https://github.com/sanger-tol/blobtoolkit

@priyanka-surana
Copy link
Contributor

I am a bit concerned about the current commit history. Seems like the rebase did not happen correctly. @zb32 is working on it again.

@alxndrdiaz
Copy link
Collaborator Author

alxndrdiaz commented Oct 28, 2022

I am a bit concerned about the current commit history. Seems like the rebase did not happen correctly. @zb32 is working on it again.

I was not sure about to push the most recent changes, but I created a local branch and will include the changes that I have made once the rebase is fixed.

@alxndrdiaz
Copy link
Collaborator Author

alxndrdiaz commented Nov 17, 2022

I am a bit concerned about the current commit history. Seems like the rebase did not happen correctly. @zb32 is working on it again.

Steps for busco_ar34 branch rebase, following git-rebase documentation.

from branch busco_ar34 create a new branch locally (and on GitHub):

git checkout -b busco_rebase
git pull
git branch --set-upstream-to=origin/busco_rebase busco_rebase

create local fix_dev branch

git checkout --track origin/fix_dev

go back to busco_rebase branch

git checkout busco_rebase

rebase

git rebase fix_dev busco_rebase

it fails with error:

error: Failed to merge in the changes.
Patch failed at 0001 restored busco_subworkflow branch
hint: Use 'git am --show-current-patch' to see the failed patch
Resolve all conflicts manually, mark them as resolved with
"git add/rm <conflicted_files>", then run "git rebase --continue".
You can instead skip this commit: run "git rebase --skip".
To abort and get back to the state before "git rebase", run "git rebase --abort". 

use hint

git am --show-current-patch

hint output

commit 0e1b1df030e6849d95325ba186233b2083a722b5
Author: alxndrdiaz <ra.ramos.diaz@gmail.com>
Date:   Thu Sep 8 15:34:56 2022 -0600
    restored busco_subworkflow branch
.....

then check status

git status

rebase in progress; onto da56a84
You are currently rebasing branch 'busco_rebase' on 'da56a84'.
  (fix conflicts and then run "git rebase --continue")
  (use "git rebase --skip" to skip this patch)
  (use "git rebase --abort" to check out the original branch)

Changes to be committed:
  (use "git restore --staged <file>..." to unstage)
	new file:   modules/local/goat_taxon_search.nf
	new file:   modules/nf-core/modules/busco/main.nf
	new file:   modules/nf-core/modules/busco/meta.yml
	new file:   modules/nf-core/modules/custom/dumpsoftwareversions/main.nf
	new file:   modules/nf-core/modules/custom/dumpsoftwareversions/meta.yml
	new file:   modules/nf-core/modules/custom/dumpsoftwareversions/templates/dumpsoftwareversions.py
	new file:   modules/nf-core/modules/multiqc/main.nf
	new file:   modules/nf-core/modules/multiqc/meta.yml
	new file:   modules/nf-core/modules/samtools/view/main.nf
	new file:   modules/nf-core/modules/samtools/view/meta.yml
	new file:   subworkflows/local/busco_diamond_blastp.nf

Unmerged paths:
  (use "git restore --staged <file>..." to unstage)
  (use "git add <file>..." to mark resolution)
	both added:      .github/PULL_REQUEST_TEMPLATE.md
	both added:      .github/workflows/linting.yml
	both added:      .prettierignore
	both added:      README.md
	both added:      assets/email_template.txt
	both added:      assets/samplesheet.csv
	both added:      assets/schema_input.json
	both added:      bin/tol_input.sh
	both added:      conf/modules.config
	both added:      conf/test.config
	both added:      docs/usage.md
	both added:      lib/NfcoreTemplate.groovy
	both added:      modules.json
	both added:      nextflow_schema.json
	both added:      subworkflows/local/input_check.nf
	both added:      workflows/blobtoolkit.nf

Then used git rebase --continue to go through each failing patch and fix conflict files, until git status output is:

On branch busco_rebase
Your branch and 'origin/busco_rebase' have diverged,
and have 122 and 116 different commits each, respectively.
  (use "git pull" to merge the remote branch into yours)
nothing to commit, working tree clean

Then used git pull, but found more conflicts:

On branch busco_rebase
Your branch and 'origin/busco_rebase' have diverged,
and have 122 and 116 different commits each, respectively.
  (use "git pull" to merge the remote branch into yours)

You have unmerged paths.
  (fix conflicts and run "git commit")
  (use "git merge --abort" to abort the merge)

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   conf/test.config
	added by us:     modules/nf-core/samtools/view/main.nf
	added by us:     modules/nf-core/samtools/view/meta.yml
	both added:      subworkflows/local/busco_diamond_blastp.nf

no changes added to commit (use "git add" and/or "git commit -a")

Fixed all conflicts then used git push to update the changes in the busco_rebase remote branch:

Enumerating objects: 464, done.
Counting objects: 100% (464/464), done.
Delta compression using up to 4 threads
Compressing objects: 100% (382/382), done.
Writing objects: 100% (437/437), 45.75 KiB | 1.48 MiB/s, done.
Total 437 (delta 204), reused 1 (delta 0)
remote: Resolving deltas: 100% (204/204), completed with 10 local objects.
To https://github.com/sanger-tol/blobtoolkit.git
   ccccdd6..de9facd  busco_rebase -> busco_rebase

@muffato
Copy link
Member

muffato commented Nov 18, 2022

At the first glance, the rebase looks mostly OK. But why did you then merge busco_ar34 in it ? The whole point is to have a single, clean, branch, with all the commits and no merge in it.

But more importantly, the busco_rebase branch doesn't pass the nextflow run CI. Even locally, the test profile doesn't start with its default parameters. Can you fix it ?

Note: you rebased on top of fix_dev, which doesn't pass the linting CI. I could perhaps fix the latter and merge #35 first ? Then you'd have to rebase on top of the new dev.

@alxndrdiaz
Copy link
Collaborator Author

alxndrdiaz commented Nov 22, 2022

At the first glance, the rebase looks mostly OK. But why did you then merge busco_ar34 in it ? The whole point is to have a single, clean, branch, with all the commits and no merge in it.

But more importantly, the busco_rebase branch doesn't pass the nextflow run CI. Even locally, the test profile doesn't start with its default parameters. Can you fix it ?

Note: you rebased on top of fix_dev, which doesn't pass the linting CI. I could perhaps fix the latter and merge #35 first ? Then you'd have to rebase on top of the new dev.

  • I just wanted to check that the rebase worked, but it would be better to just merge with busco_ar34.
  • It would be easier to just debug fix_dev then try to rebase or merge busco_ar34. I will work on it, if you have any other suggestions please let me know.

@muffato
Copy link
Member

muffato commented Nov 22, 2022

Like Priyanka said, ignore Zaynab's for now, and just rebase your own changes onto dev.
The key is that nextflow run . -profile test,singularity without any other command-line arguments must still run without any problem on your branch. That'll be the first condition for merging the new branch.

@alxndrdiaz alxndrdiaz added duplicate This issue or pull request already exists wontfix This will not be worked on labels Nov 25, 2022
@alxndrdiaz
Copy link
Collaborator Author

Like Priyanka said, ignore Zaynab's for now, and just rebase your own changes onto dev. The key is that nextflow run . -profile test,singularity without any other command-line arguments must still run without any problem on your branch. That'll be the first condition for merging the new branch.

I will close this PR and will open a new one with the rebased branch.

@alxndrdiaz alxndrdiaz closed this Nov 25, 2022
@alxndrdiaz alxndrdiaz deleted the busco_subworkflow branch January 23, 2023 22:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate This issue or pull request already exists enhancement New feature or request help wanted Extra attention is needed wontfix This will not be worked on
Projects
Development

Successfully merging this pull request may close these issues.

subworkflow: diamond_blastp subworkflow: busco
4 participants