Repository navigation
Feature/euks support - #104
Open
KateSakharova wants to merge 27 commits into
Open
KateSakharova wants to merge 27 commits into
KateSakharova wants to merge 27 commits into
Conversation
KateSakharova
marked this pull request as draft
October 14, 2026 17:06
|
❌ nf-test failed with latest Nextflow versionNote Tests with Nextflow's latest version failed but it will not cause a CI workflow failure.
See the full run for details. |
KateSakharova
commented
Oct 20, 2026
dezordi
reviewed
Oct 21, 2026
dezordi
left a comment
dezordi
left a comment
There was a problem hiding this comment.
Nice work Kate! I put some suggestions and questions, and also I would like to known if you have you considered, at any point, keeping genome_evaluation as two separate subworkflows (prok and euks)?
I know there are some functions shared between the two, and keeping them together allows for code reuse, however, in my opnion, this makes long-term maintenance more difficult. It also means that submissions for prokaryotes might receive parameters intended for eukaryotes, and vice versa.
Finally, I noticed that in cases where eukcc fails to estimate completeness and contamination, the workflow continues and attempts to submit genomes with NA values for these metrics, is that correct? Perhaps we should consider a way to exclude such genomes from the submission pipeline while keeping a record of them in the logs.
|
|
||
| Before classification, input FASTA files are normalized to a `.fasta` suffix by `RENAME_FASTA_FOR_CATPACK`. Classification is then run in bin mode (`CAT_pack bins`, followed by `CAT_pack add_names`). | ||
|
|
||
| Database input is taken from `--cat_db` when provided; otherwise the workflow builds CAT_pack database from NCBI non-redundant protein database (`nr`). See [CAT_pack documentation](https://github.com/MGXlab/CAT_pack#downloading-preconstructed-database-files). There are two options available: `nr` - NCBI non-redundant protein database and `gtdb` (Genome Taxonomy Database (GTDB) proteins). We use NCBI proteins because ENA supports only NCBI format of taxonomy for submission. The resulting classification table is parsed, and the lineage field is written to `NCBI_lineage`. |
There was a problem hiding this comment.
when we run CAT for euks, it uses prodigal which can raise this error for long contigs hyattpd/Prodigal#54
This could entail a limitation on the submission of eukaryotic genomes. I see three alternatives here:
- Leave it as is, which might block the submission of certain genomes.
- Run
cat_pacusing proteins (see example: https://github.com/EBI-Metagenomics/genomes-catalogue-pipeline/blob/fix/euks-move-bat-invocation/modules/bat.nf#L20), which requires to have the proteins somehow. - Run
eukcc, which generates a lineage for eukaryotes and identifies them as such.
Contributor
Author
There was a problem hiding this comment.
good point!
I wanted to run "universal" tool and then make a decision. If I will run eukcc on prokaryotes it might give weird result, no?
I also thought to run metaeuk separately and pass those proteins to eukcc (would it work with --AA?) and to cat_pack. But ideally we also should not run metaeuk for proks? :confuced:
There was a problem hiding this comment.
Yes, it’s a complicated issue... EukCC would return results with NA metrics for proks, and MetaEuk would require a reference database for "easy predict" (since it’s only for taxonomy, I imagine the SCMG database from EukCC might work), but it wouldn’t be ideal for prokaryotes either...
There was a problem hiding this comment.
It might be worth bringing this adjustment into the nf-core module, so it can also be tested across other executors and also avoid this bug in other pipelines
KateSakharova
commented
Oct 21, 2026
KateSakharova
commented
Oct 21, 2026
|
Warning Newer version of the nf-core template is available. Your pipeline is using an old version of the nf-core template: 4.1.0. For more documentation on how to update your pipeline, please see the Synchronisation documentation. |
KateSakharova
marked this pull request as ready for review
October 26, 2026 21:03
ochkalova
requested changes
Oct 28, 2026
Comment on lines
+12
to
+18
| // ignore euk_mags_quality assessment because Eukcc DB is big for GA | ||
| // ignore genome_evaluation_proks test because checkm2 container is too big | ||
| ignore = [ | ||
| 'modules/nf-core/**/tests/*', | ||
| 'subworkflows/nf-core/**/tests/*', | ||
| 'tests/mag_no_quality_euk_without_ena_interaction.nf.test', | ||
| 'subworkflows/local/genome_evaluation_proks/tests/*' |
Contributor
There was a problem hiding this comment.
Nice! I never knew it can be done like this
| "type": "string", | ||
| "default": null, | ||
| "description": "Tool, including version, that was used to calculate completeness and contamination, e.g. CheckM2_v1.0.0", | ||
| "description": "Tool, including version, that was used to calculate completeness and contamination, e.g. CheckM2_v1.0.0. Several tools are separated by `;`, e.g. busco_v6.1.0;eukcc_v2.2.0. If the pipeline runs additional tools for this genome, they are appended to the provided value.", |
Contributor
There was a problem hiding this comment.
Ouch, I wonder if this can cause weird-looking metadata 🤔
Probably not, because the only case when we append is when eukcc is calculated in addition to busco, right?
Contributor
Author
There was a problem hiding this comment.
or it can be the case when user has completeness and specified the tool, but no contamination. We will have to calculate contamination and basically leave both tools in metadata
| ], | ||
| "default": null, | ||
| "description": "Presence or absence of the 23S, 16S, and 5S rRNA genes and at least 18 tRNAs. This is used for MISAG/MIMAG assembly quality classification. Options: 'true' or 'false'", | ||
| "description": "Presence or absence of the 23S, 16S, and 5S rRNA genes (prokaryotes) or the 28S, 18S, 5.8S and 5S rRNA genes (eukaryotes) and at least 18 tRNAs. This is used for MISAG/MIMAG assembly quality classification. Options: true or false.", |
Contributor
There was a problem hiding this comment.
I have no idea if it's really specified in mimag for euks. ENA explains it only for proks 🤔
We need to ask them next Monday
There was a problem hiding this comment.
I asked in the past, criteria for euks are not as well defined, since they are so difficult to assemble. Many people assume the same criteria as proks, some other decrease thresholds, but I think things might be changing soon and ENA might not have a precise answer
Contributor
Author
There was a problem hiding this comment.
my AI friends told me to just say True in case any of RNA present, not all like in proks because again those are quite complecated...
| } | ||
|
|
||
| withName: 'TRNASCANSE' { | ||
| ext.args = { [euk: '-E', arc: '-A', bac: '-B'].get(meta.domain, '-G') } |
Contributor
Author
There was a problem hiding this comment.
Basic Options
-E : search for eukaryotic tRNAs (default)
-B : search for bacterial tRNAs
-A : search for archaeal tRNAs
-M <model> : search for mitochondrial tRNAs
options: mammal, vert
-O : search for other organellar tRNAs
-G : use general tRNA model (cytoslic tRNAs from all 3 domains included)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Working on #40
Current steps order in GENOMESUBMIT:
meta.domainassigned toarc/bac/eukdepending on taxaext.args = { [euk: '-E', arc: '-A', bac: '-B'].get(meta.domain, '-G') }what set to use depending on domain[meta, fasta, meta.domain]eukaryotic_completeness_tool. Default:eukcc. If that arg would be set to "busco" then BUSCO would be used as completeness. Contamination always set with EukCC. If contamination is provided by user - it is not assigned by pipeline. Stats tools are concatenated with;In addition:
-stubtest for genome_evaluation because it requires databasesTests:
Added eukaryotic genome to
no_rnatest. Now that test is running 2 genomes.Added test for
euk_qualitywith small eukcc_db created only for fungi (but removed it from GA because DB is still big)Removed any ENA interaction in tests (
REGISTERSTUDY|SUBMIT|CREATE_MANIFESTS):mag_no_rna_presence.nf.testmag_no_taxonomy.nf.testmag_no_quality_euk.nf.testmag_no_coverage_pe.nf.testmag_no_coverage_se.nf.testRenamed those tests to
no_ena_queriesto better reflect changesAdditionally:
--test_upload=false && --webincli_mode=validateand added notes to docs releated to --webincli_mode validate will include study and co assembly sample regsitration #118TODO:
Check with ENA about RNA rules for euks