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

Resolving tiny ciriquant bug #109

Merged
merged 4 commits into from
Apr 15, 2024
Merged

Conversation

MariekeVromman
Copy link
Contributor

Recently I've encountered two bugs when running ciriquant.

One is described in more detail in this issue (see mostly the second comment). In short, the yml file where the hisat2 index is specified did not contain the correct index name anymore after another commit in the pipeline.

I am not very familiar with how the pipeline works, so in this PR I just reverted the change. This seems to do the trick, but I'm not sure if this is good practice. I'm assuming there was a good reason to change the way the hisat2_prefix was set in the previous commit, so I would be glad if anyone could explain!

A second issue I encountered, was this error message also during the CIRIQUANT process:
[Thu 2024-04-11 10:25:54] [INFO ] Running CIRI2 for circRNA detection .. Traceback (most recent call last): File "/usr/local/bin/CIRIquant", line 10, in <module> sys.exit(main()) File "/usr/local/lib/python2.7/site-packages/CIRIquant/main.py", line 183, in main circ_parser.convert(bed_file) File "/usr/local/lib/python2.7/site-packages/CIRIquant/utils.py", line 239, in convert circ_data = getattr(self, '_' + self.tool.lower())() File "/usr/local/lib/python2.7/site-packages/CIRIquant/utils.py", line 146, in _ciri2 with open(self.circ, 'r') as f: IOError: [Errno 2] No such file or directory: '/Users/marieke/Documents/ciriquant/work/4d/1836ef6340f9708ba469d4a289e0ca/fust1_1/circ/fust1_1.ciri'

I've solved this by just adding an mkdir and touch command to create the necessary directories and file in the CIRIQUANT process. Again, it looks like it works, but I'm not sure if this is good practice, so any advise is welcome. The output files seem 'normal'.

This can all be tested by running:
nextflow run circrna -profile test,docker --tool ciriquant

Copy link

github-actions bot commented Apr 11, 2024

nf-core lint overall result: Passed ✅ ⚠️

Posted for pipeline commit cf0a055

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

❗ Test warnings:

  • readme - README contains the placeholder zenodo.XXXXXXX. This should be replaced with the zenodo doi (after the first release).

✅ Tests passed:

Run details

  • nf-core/tools version 2.13.1
  • Run at 2024-04-11 14:05:16

@MariekeVromman MariekeVromman linked an issue Apr 11, 2024 that may be closed by this pull request
@MariekeVromman MariekeVromman changed the title Resolving 2 ciriquant bugs - NOT SURE IF CORRECT - PLEASE ADVISE Resolving 2 ciriquant bugs - PLEASE ADVISE Apr 11, 2024
@nictru
Copy link
Contributor

nictru commented Apr 11, 2024

  1. I added the meta fields to the input in order to give it the same interface as most other processes have. I am not sure why I made the change in line 22, I think in my configuration the fasta file was called fasta.fa and I most likely just assumed that this will always be the case which is obviously wrong. Looking at the way the hisat2 index is created here, I think the cleanest way of solving this would be with hisat2_prefix = fasta.baseName

@nictru
Copy link
Contributor

nictru commented Apr 11, 2024

  1. Confuses me a bit as this file was last changed in 2022 and if this would really be the problem, then it could not have worked since back then. Maybe it's due to a certain environment state or something

@MariekeVromman
Copy link
Contributor Author

Thanks for the advice! The first indeed also works with `hisat2_prefix = fasta.baseName``

For the second bug, I had encountered this bug a couple of times over the last months, but did not see a 'pattern'.
I removed the mkdir+touch changes to make it easier to test.

when using this branch and running:
nextflow run circrna -profile test,docker --tool ciriquant

locally I get

[Thu 2024-04-11 14:02:11] [INFO ] Running CIRI2 for circRNA detection .. Traceback (most recent call last): File "/usr/local/bin/CIRIquant", line 10, in <module> sys.exit(main()) File "/usr/local/lib/python2.7/site-packages/CIRIquant/main.py", line 183, in main circ_parser.convert(bed_file) File "/usr/local/lib/python2.7/site-packages/CIRIquant/utils.py", line 239, in convert circ_data = getattr(self, '_' + self.tool.lower())() File "/usr/local/lib/python2.7/site-packages/CIRIquant/utils.py", line 146, in _ciri2 with open(self.circ, 'r') as f: IOError: [Errno 2] No such file or directory: 'fust1_1/circ/fust1_1.ciri'

on the cluster using this command:

/data/users/mvromman/software/nextflow/nextflow run \ /data/tmp/mvromman/circrna \ -profile singularity,test \ -c /data/users/mvromman/projects/06_TNBC_circ/20240322_nextflow.config \ --tool ciriquant

it gives no errors, so it must be something else.
I'm going to see if I can find where it comes from, but I'll leave it out of this PR, as I'm leaving on holiday tomorrow for 10 days, so it won't be soon.

@MariekeVromman MariekeVromman changed the title Resolving 2 ciriquant bugs - PLEASE ADVISE Resolving tiny ciriquant bug Apr 11, 2024
@MariekeVromman
Copy link
Contributor Author

@nictru, could you approve the first change, if you think it's okay? Thanks a lot!

@nictru
Copy link
Contributor

nictru commented Apr 15, 2024

We can merge this one for now and check the other one later on. If the fix for the second one does not break anything, we can also just include it in the pipeline, but maybe we should have a second look at this first.

@nictru nictru merged commit d4199c4 into nf-core:dev Apr 15, 2024
4 checks passed
@MariekeVromman MariekeVromman deleted the ciriquant_bug branch April 22, 2024 07:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CIRIquant test fails since commit 72dd514, PR #104
3 participants