Skip to content

Conversation

@SusiJo
Copy link

@SusiJo SusiJo commented Sep 11, 2025

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/metapep branch on the nf-core/test-datasets repository.
  • Make sure your code lints (nf-core pipelines 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
Copy link

github-actions bot commented Sep 11, 2025

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

Posted for pipeline commit bb7fdf9

+| ✅ 223 tests passed       |+
#| ❔   2 tests were ignored |#
!| ❗  23 tests had warnings |!

❗ Test warnings:

  • files_exist - File not found: conf/igenomes_ignored.config
  • readme - README contains the placeholder zenodo.XXXXXXX. This should be replaced with the zenodo doi (after the first release).
  • schema_description - No description provided in schema for parameter: igenomes_ignore
  • local_component_structure - finalize_microbiome_entities.nf in modules/local should be moved to a TOOL/SUBTOOL/main.nf structure
  • local_component_structure - predict_epitopes.nf in modules/local should be moved to a TOOL/SUBTOOL/main.nf structure
  • local_component_structure - unify_model_lengths.nf in modules/local should be moved to a TOOL/SUBTOOL/main.nf structure
  • local_component_structure - download_proteins.nf in modules/local should be moved to a TOOL/SUBTOOL/main.nf structure
  • local_component_structure - unpack_bin_archives.nf in modules/local should be moved to a TOOL/SUBTOOL/main.nf structure
  • local_component_structure - create_protein_tsv.nf in modules/local should be moved to a TOOL/SUBTOOL/main.nf structure
  • local_component_structure - prepare_score_distribution.nf in modules/local should be moved to a TOOL/SUBTOOL/main.nf structure
  • local_component_structure - split_pred_tasks.nf in modules/local should be moved to a TOOL/SUBTOOL/main.nf structure
  • local_component_structure - generate_protein_and_entity_ids.nf in modules/local should be moved to a TOOL/SUBTOOL/main.nf structure
  • local_component_structure - merge_predictions_buffer.nf in modules/local should be moved to a TOOL/SUBTOOL/main.nf structure
  • local_component_structure - prepare_entity_binding_ratios.nf in modules/local should be moved to a TOOL/SUBTOOL/main.nf structure
  • local_component_structure - collect_stats.nf in modules/local should be moved to a TOOL/SUBTOOL/main.nf structure
  • local_component_structure - merge_predictions.nf in modules/local should be moved to a TOOL/SUBTOOL/main.nf structure
  • local_component_structure - check_samplesheet_create_tables.nf in modules/local should be moved to a TOOL/SUBTOOL/main.nf structure
  • local_component_structure - plot_entity_binding_ratios.nf in modules/local should be moved to a TOOL/SUBTOOL/main.nf structure
  • local_component_structure - plot_score_distribution.nf in modules/local should be moved to a TOOL/SUBTOOL/main.nf structure
  • local_component_structure - assign_nucl_entity_weights.nf in modules/local should be moved to a TOOL/SUBTOOL/main.nf structure
  • local_component_structure - generate_peptides.nf in modules/local should be moved to a TOOL/SUBTOOL/main.nf structure
  • local_component_structure - epytope_show_supported_models.nf in modules/local should be moved to a TOOL/SUBTOOL/main.nf structure
  • local_component_structure - process_input.nf in subworkflows/local should be moved to a SUBWORKFLOW_NAME/main.nf structure

❔ Tests ignored:

  • files_exist - File is ignored: conf/igenomes.config
  • files_unchanged - File ignored due to lint config: .gitignore or .prettierignore

✅ Tests passed:

Run details

  • nf-core/tools version 3.3.2
  • Run at 2025-11-12 12:26:55

@SusiJo SusiJo force-pushed the nf-core-template-merge-3.3.2 branch 9 times, most recently from 5648a98 to 6fd50eb Compare September 24, 2025 08:14
@SusiJo SusiJo force-pushed the nf-core-template-merge-3.3.2 branch from 707f034 to db78bce Compare October 14, 2025 11:55
@CaroAMN
Copy link

CaroAMN commented Oct 14, 2025

Looks good to me 🙂 👍

Copy link
Member

@skrakau skrakau left a comment

Choose a reason for hiding this comment

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

Thanks a lot for updating this! I would have a two question or remarks:

  • could you squash your commits at the end? that would keep the commit history clean as there are multiple back-and-forth changes that would be confusing otherwise
  • why did you replace the snapshots from the peptides, proteins and predictions db files and replace it with checking for the number of lines?

The rest looks good!

@SusiJo
Copy link
Author

SusiJo commented Oct 17, 2025

Hi,
thanks for the feedback. There is no squash commit enabled here directly, but I can of course do that on the command line.

I changed the file checks for the gzipped files because at some point nf-test created different md5sums when testing locally vs in the ci runners. Checking for the exact number of lines aimed at making sure those files contain the same number of peptides/proteins while ignoring the gzipped headers that can differ.
If you prefer the md5sum check I can also switch this back, but re-creating the snapshots will take a few hours.

@skrakau
Copy link
Member

skrakau commented Oct 17, 2025

Did it work to check the differences between the files?

@nf-core-bot
Copy link
Member

Warning

Newer version of the nf-core template is available.

Your pipeline is using an old version of the nf-core template: 3.3.2.
Please update your pipeline to the latest version.

For more documentation on how to update your pipeline, please see the nf-core documentation and Synchronisation documentation.

@SusiJo
Copy link
Author

SusiJo commented Oct 20, 2025

Did it work to check the differences between the files?

There are minor floating differences between the predictions.tsv.gz for test_mhcflurry. I checked the file content for 10 random lines and there are 2 differing lines.

Here's an example:

Tested locally                                            CI run
"26069\t0.1018050710360207                "26069\t0.1018050710360209               # line 1010
"79667\t0.0714185908436774               "79667\t0.0714185908436775                # line 1013

Copy link
Member

@skrakau skrakau left a comment

Choose a reason for hiding this comment

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

nf-tests still require change

@SusiJo SusiJo force-pushed the nf-core-template-merge-3.3.2 branch from 7442843 to 429592a Compare November 7, 2025 13:30
@SusiJo
Copy link
Author

SusiJo commented Nov 11, 2025

The profiles: test_all, test_taxa_all, test_taxa_specific_only, test_mouse download data from NCBI/Entrez.
The template update md5sums are stable across different infrastructures and in CI runners.
They are however different compared to the latest checksums. The profiles use taxa input files without fixed assembly ids:

If assemblies get updated on NCBI the files will not be reproducible.

Therefore the md5sums will be removed for now from the snapshots of the 4 affected profiles. Instead we will only test for existence of those files.

@SusiJo SusiJo force-pushed the nf-core-template-merge-3.3.2 branch from 337ea4d to 4c95a39 Compare November 12, 2025 07:39
@skrakau
Copy link
Member

skrakau commented Nov 12, 2025

Thanks for squashing the commits! :) This makes it much easier to only check metapep test related changes together independently of all the template update changes 👍

{ assert snapshot(path("$outputDir/db_tables/alleles.tsv"),
path("$outputDir/db_tables/conditions_alleles.tsv"),
path("$outputDir/db_tables/conditions.tsv"),
path("$outputDir/db_tables/entities_proteins.tsv"),
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't here microbiomes_entities.tsv be added (because the entities, i.e. taxa, themselves do not change), while entities_proteins.tsv would be expected to change? That's what we listed in our meeting, or is this not what you observed in your tests?

Copy link
Author

Choose a reason for hiding this comment

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

microbiomes_entities.tsv has a different md5sum compared to the last release. It is not clear why this happens.
I will put entities_proteins.tsv to the list of files to be only checked for existence.

@SusiJo SusiJo force-pushed the nf-core-template-merge-3.3.2 branch from 4c95a39 to bb7fdf9 Compare November 12, 2025 12:25
@SusiJo SusiJo requested a review from skrakau November 12, 2025 14:37
@skrakau
Copy link
Member

skrakau commented Nov 13, 2025

I changed the file checks for the gzipped files because at some point nf-test created different md5sums when testing locally vs in the ci runners. Checking for the exact number of lines aimed at making sure those files contain the same number of peptides/proteins while ignoring the gzipped headers that can differ

Just for completeness, it turned out that nf-test can handle gzipped files and that the differences here had other reasons.

Copy link
Member

@skrakau skrakau left a comment

Choose a reason for hiding this comment

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

Thanks @SusiJo 🎉 🚀
This was a lot of hidden work, but it looks great now!

@SusiJo SusiJo merged commit 4af8299 into dev Nov 13, 2025
20 checks passed
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.

5 participants