Skip to content

STRY0020932: metadatatransformation: Migrate nf-validation to nf-schema - #32

Open
emarinier wants to merge 16 commits into
devfrom
nf-schema
Open

STRY0020932: metadatatransformation: Migrate nf-validation to nf-schema#32
emarinier wants to merge 16 commits into
devfrom
nf-schema

Conversation

@emarinier

@emarinier emarinier commented Jul 6, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Converts from nf-validation to nf-schema and from CSV sample sheets to JSON sample sheets.

Description

As a pipeline developer, I would like the metadatatransformation pipeline to be updated to use the newer nf-schema Nextflow plugin, so that we are keeping up-to-date with Nextflow standards.

Acceptance Criteria

  • Migrate nf-validation to nf-schema
  • Update pipeline so that it supports "samplesheet.json" in addition to "samplesheet.csv". See Added samplesheet.json and tests by apetkau · Pull Request #18 · phac-nml/iridanextexample2 for details on changes needed
    • Update "pattern" in "nextflow_schema" so that CSV and JSON files are accepted for "--input"
    • Add a test case where a "samplesheet.json" is passed instead of a CSV file.
    • NOTE: CSV SUPPORT REMOVED!
  • Existing tests should continue to work
  • Update pipelines.json configuration ("definitions" to "$defs") so that the pipeline loads properly in IRIDA Next for DEV and TEST

Remarks

I needed to entirely convert from using CSV sample sheets to JSON sample sheets. The problem with CSV sample sheets seems to be that the part of nf-schema that parses and loads the sample sheet, doesn't know what data type the input is supposed to be (integer, number/float, string, etc.), then when it gets to the validator, a mismatch in data types can cause it to fail.

For example:

schema_input.json

"metadata_1": {
    "type": ["string"],
    [...]
},

sample sheet

sample,sample_name,metadata_1,metadata_2,metadata_3,metadata_4,metadata_5,metadata_6,metadata_7,metadata_8
sample1,"ABC","1.1","1.2","1.3","1.4","1.5","1.6","1.7","1.8"
sample2,"DEF","2.1","2.2","2.3","2.4","2.5","2.6","2.7","2.8"
sample3,"GHI","3.1","3.2","3.3","3.4","3.5","3.6","3.7","3.8"

error

Entry 1: Error for field 'metadata_1' (1.1): Value is [number] but should be [string] (Metadata associated with the sample (metadata_1).)

lenientMode did not work, and the recommendation I found was to switch to using JSON inputs, because JSON encodes some data types in the sample sheet itself. For example:

[
  {
    "sample": "sample1",
    "sample_name": "ABC",
    "metadata_1": "1.1",
  }
]

The fact that it's "1.1" and not 1.1 encodes that the value should be interpreted as a string. It therefore loads the data from the JSON sample sheet correctly and validates.

However, this will mean that the pipelines will no longer support CSV sample sheet inputs, and will be incompatible with IRIDA Next until they make changes to support this. The recommendation I found to solve this problem with data types and validation was to switch from CSV to JSON.

I also observed problems with nf-schema loading string IDs as floats, but the float precision was handled poorly, and the string was mangled. 1.7 -> 1.7000000000000002. Having the data load in this way as a string, rather than having metadata_N support multiple input types ("type": ["string"]), might be the way to go by default, unless we have a very good reason to have non-string types in a pipeline.

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
  • Make sure your code lints (nf-core 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).

@emarinier emarinier self-assigned this Jul 6, 2026
emarinier
emarinier commented Jul 6, 2026
View reviewed changes
Comment thread assets/schema_input.json
"errorMessage": "Metadata associated with the sample (metadata_1).",
"default": "",
"pattern": "^[^\\n\\t\"]+$"
"pattern": "^[^\\n\\t\"]*$"

@emarinier emarinier Jul 6, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

When the JSON sample sheet contained entries like "", then this failed, because the + says there needs to be one or more things, but when loaded from the JSON, there can be zero.

emarinier
emarinier commented Jul 6, 2026
View reviewed changes
Comment thread tests/pipelines/age_pnc.nf.test
assert results.text.contains("junk_3,j_3,0,0,0,0,,False,The date format does not match the expected format (YYYY-MM-DD); invalid host_age_unit (0)")
assert results.text.contains("junk_4,j_4,0000-00-00,0000-00-00,0000-00-00,0000-00-00,,False,The date format does not match the expected format (YYYY-MM-DD); host_age (0000-00-00) could not be converted to a number")
assert results.text.contains("junk_5,j_5,1.0,2.0,3.0,4.0,,False,The date format does not match the expected format (YYYY-MM-DD); invalid host_age_unit (4.0)")
assert results.text.contains("junk_5,j_5,1,2,3,4,,False,The date format does not match the expected format (YYYY-MM-DD); invalid host_age_unit (4)")

@emarinier emarinier Jul 6, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The original data was 4, it was getting parsed in as 4.0 and output as such.

emarinier
emarinier commented Jul 6, 2026
View reviewed changes
Comment thread nextflow.config
// Nextflow plugins
plugins {
id 'nf-validation@1.1.3' // Validation of pipeline parameters and creation of an input channel from a sample sheet
id 'nf-schema@2.4.2'

@emarinier emarinier Jul 6, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

See offline discussion or ask offline if curious for why we need this version of nf-schema, rather than the newer version.

emarinier
emarinier commented Jul 6, 2026
View reviewed changes
Comment thread nextflow_schema.json
"exists": true,
"mimetype": "text/csv",
"pattern": "^\\S+\\.csv$",
"pattern": "^\\S+\\.json$",

@emarinier emarinier Jul 6, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

As described in the PR description, we cannot support CSV sample sheets because the part of nf-schema that loads the data types conflicts with the part that validates for strings that look like numbers.

@emarinier emarinier changed the title Nf schema STRY0020932: metadatatransformation: Migrate nf-validation to nf-schema Jul 6, 2026
@github-actions

github-actions Bot commented Jul 8, 2026 •
edited
Loading

Copy link
Copy Markdown

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

Posted for pipeline commit bf4607c

+| ✅ 156 tests passed       |+
#| ❔  19 tests were ignored |#
!| ❗   7 tests had warnings |!
Details

❗ Test warnings:

  • nextflow_config - Config variable not found: validation.help.beforeText
  • nextflow_config - Config variable not found: validation.help.afterText
  • nextflow_config - Config variable not found: validation.help.command
  • nextflow_config - Config variable not found: validation.summary.beforeText
  • nextflow_config - Config variable not found: validation.summary.afterText
  • nextflow_config - Config manifest.version should end in dev: 1.4.2
  • readme - README did not have an nf-core template version badge.

❔ Tests ignored:

✅ Tests passed:

Run details

  • nf-core/tools version 3.3.2
  • Run at 2026-07-08 16:11:31

emarinier
emarinier commented Jul 8, 2026
View reviewed changes
Comment thread nextflow.config
hook_url = null
help = false
version = false
trace_report_suffix = new java.util.Date().format( 'yyyy-MM-dd_HH-mm-ss')// Config options

@emarinier emarinier Jul 8, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

These config changes were causing linting failures. It looks like the way of doing things like declaring variables has changed in more recent versions of Nextflow, so this was needed to update it.

emarinier
emarinier commented Jul 8, 2026
View reviewed changes
Comment thread conf/test.config

// Input data
input = 'https://raw.githubusercontent.com/phac-nml/metadatatransformation/main/assets/samplesheet.csv'
input = 'https://raw.githubusercontent.com/phac-nml/metadatatransformation/nf-schema/assets/samplesheet.json'

@emarinier emarinier Jul 8, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Will need to change this to main later.

@emarinier
emarinier requested a review from apetkau August 4, 2026 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@apetkau apetkau Awaiting requested review from apetkau

Assignees

@emarinier emarinier

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@emarinier