Skip to content

Release 2.1.0#149

Merged
johandahlberg merged 75 commits into
masterfrom
dev
Sep 17, 2025
Merged

Release 2.1.0#149
johandahlberg merged 75 commits into
masterfrom
dev

Conversation

@johandahlberg

Copy link
Copy Markdown
Collaborator

Release nf-core/pixelator 2.1.0.

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/pixelator 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).

nf-core-bot and others added 30 commits June 3, 2025 11:01
Co-authored-by: Florian De Temmerman <69114541+fbdtemme@users.noreply.github.com>
…d-denoise-step

Feature/pna 1036 add denoise step
…me-and-metromap

Update README and metromap
@github-actions

github-actions Bot commented Sep 1, 2025

Copy link
Copy Markdown

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

Posted for pipeline commit f4fee3b

+| ✅ 291 tests passed       |+
#| ❔   7 tests were ignored |#
!| ❗   5 tests had warnings |!
Details

❗ Test warnings:

  • files_unchanged - LICENSE does not match the template
  • pipeline_todos - TODO string in main.nf: Optionally add in-text citation tools to this list.
  • pipeline_todos - TODO string in main.nf: Optionally add bibliographic entries to this list.
  • pipeline_todos - TODO string in main.nf: Only uncomment below if logic in toolCitationText/toolBibliographyText has been filled!
  • included_configs - Pipeline config does not include custom configs. Please add the includeConfig line.

❔ Tests ignored:

✅ Tests passed:

Run details

  • nf-core/tools version 3.3.2
  • Run at 2025-09-17 15:11:41

@johandahlberg

Copy link
Copy Markdown
Collaborator Author

It seems the failing pipeline download tests here are caused by this bug: nf-core/tools#3742

Comment thread CHANGELOG.md Outdated
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [[2.0.0](https://github.com/nf-core/pixelator/releases/tag/2.0.0)] - 2025-06-23
## [[2.1.0](https://github.com/nf-core/pixelator/releases/tag/x.x.x)] - YYYY-MM-DD

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reminder to update the date and version here

@nvnieuwk nvnieuwk left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Some minor comments, but nothing blocking this release for me

Comment thread conf/modules.pna.config
withName: PIXELATOR_PNA_LAYOUT {
ext.when = { !params.skip_layout }
ext.args = {
ext.when = { !params.skip_layout }

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Try to limit the use of ext.when. This will get removed in the future in favor of if statements in the workflow itself. This is not a blocking issue for this release but it would be best to update this before the next release

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yeah. We have a bunch of these, so I think it would make sense to go over all of them in one go.


"""
# Copy the full quarto dir from the read-only image into the workdir
cp -r /workspace/inst/quarto/ ./quarto/

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This seems like it should be a "reference" input to the module instead?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

We've been trying around with a bunch of different solutions to this - in the end this was the only one that actually worked. Why is a bit a of mystery to me.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Okay fine by me then :)

Comment on lines +45 to +51
# Hard coding the version for the stub, since for some
# reason it work in nf-test otherwise.
cat <<-END_VERSIONS > versions.yml
"${task.process}":
experiment-summary: 0.4.2
END_VERSIONS
"""

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This seems strange, the only difference between a stub run and a normal run is the difference in the bash code executed. What errors did you get here?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Well, that is a good question. I could not replicate it now, so I have restored this to actually fetch the version as in the main script now.

Comment on lines +53 to +56
{assert path(process.out.report_json.get(0).get(1)).exists()},
{assert path(process.out.metadata.get(0).get(1)).exists()},
{assert path(process.out.dataset.get(0).get(1)).exists()},
{assert process.out.dataset.get(0).get(1).endsWith(".pxl")},

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This seems like a downgrade in test quality, is the output significantly different here?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

We have had a lot of issues with the tests being very brittle due to changing metadata. Since the pipeline basically only runs pixelator anytime we do an update of the pixelator version all the tests fail due to changes in the metadata of the files (that among other things contain the pixelator version run). As we have an extensive test suite on pixelator itself testing the content here has rather low return on investment for us, while causing a lot of extra work. In order to try to balance that out a bit we have decided to go for less stringent testing here. Does that make sense?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Yes thank you, it does make sense. This might be worth to create an issue about and update the tests gradually in the future to ensure reproducibility of the pipeline

Comment on lines +60 to +61
{assert path(process.out.report_json.get(0).get(1)).exists()},
{assert path(process.out.metadata.get(0).get(1)).exists()},

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Same here

Comment thread subworkflows/local/pna/generate_reports.nf Outdated
@johandahlberg johandahlberg merged commit 2433787 into master Sep 17, 2025
38 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