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

Possible bug in logic code related to mature, hairpin and species setup #414

Closed
lpantano opened this issue Sep 6, 2024 · 4 comments
Closed
Labels
bug Something isn't working

Comments

@lpantano
Copy link
Contributor

lpantano commented Sep 6, 2024

Description of the bug

For this PR I realized that I can see there are a few things that will generate problems due to the duality between

  • params.mirtrace_species
  • params.mirna_gtf
  • params.mature
  • params.hairpin
  • params.mirgenedb_*

params.mirtrace_species is used for miRTrace.
params.mirtrace_species is re-used for mirtop_quant when params.mature and params.hairpin and params.mirna_gtf are set up

all the params.mirgendb_* is used for mirtop_quant when they are set.

So the logic should be:

you set up params.mirgendb_* or params.mirna_gtf, params.mature, params.hairpin, not both.

you set up params.mirtrace_species if you want to use miRTrace or default miRBase files(params.mirna_gtf, params.mature, params.hairpin)

if user set up params.mirgendb_* and wants to run miRTrace, then params.mirtrace_species is used with miRTrace and params.mirgendb_species is used with miRTop. I don't think this is happening right.

Maybe we need a meeting to sort this out?

Command used and terminal output

No response

Relevant files

No response

System information

No response

@lpantano lpantano added the bug Something isn't working label Sep 6, 2024
@nschcolnicov
Copy link
Contributor

I think this should be fixed simply by updating the line mentioned by Lorena:

ch_mirna_gtf = val_mirna_gtf ? Channel.empty() : ( mirna_gtf_from_species ? Channel.fromPath(mirna_gtf_from_species, checkIfExists: true).collect() : Channel.empty() ) //TODO for ch_mirna_gtf, shouldn't it try to build a channel.fromPath with params.mirna_gtf, if true? (instead of setting it to empty). Is this parameter used for non mirgenedb runs?

with something like this:

ch_mirna_gtf              = val_mirna_gtf                 ?  Channel.fromPath(val_mirna_gtf, checkIfExists: true).collect() : ( mirna_gtf_from_species ? Channel.fromPath(mirna_gtf_from_species, checkIfExists: true).collect() :  Channel.empty() )

But we probably would need to update the CI tests as well, and check if there is any other issue that arises from this change

@lpantano
Copy link
Contributor Author

lpantano commented Sep 6, 2024

I think this issue is a little bit bigger scope. This is about how the pipeline setup the variables whether people want to use miRBase files or mirgeneDB, and together with miRTrace. I didn't test it but , I don't think will work well. When I run test_mirgendb config file, mirtop doesn't run, because mirtrace_species is not setup. But it should run, actually mirtrace should be able to run as well, even if people use mirgendb files to quantify.

@atrigila
Copy link
Contributor

The issue was solved in #386 but some improvements in design can be made regarding the use of these databases in the future.
Some ideas discussed are:

  • Use a single channel with hairpin, mature, gtf and species. This channel can have an associated metamap adding the origin DB (mirgeneDB or other) for tracking.
  • There could be some checks that ensure that the 3 files come from the same database.
  • The checks will also ensure that one single channel is populated and used across the whole pipeline. We could avoid having several params/channels referring to similar information. This is similar to what is done will the protocols.
  • The species can likely be "translated" from other databases to mirTrace nomenclature by removing/adding a capital letter at the beginning (e.g. "hsa" to "Hsa").
  • The gtf from each species could obtained automatically from these DBs using getGenomeAttribute() as we do with iGenomes.

@nschcolnicov
Copy link
Contributor

Closing this ticket, as the issue described was solved in #386. @lpantano Feel free to open a new one, if you think there was something else to work on

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done
Development

No branches or pull requests

3 participants