Conversation
Companion to #680. These SDRFs are re-annotated versions of the examples in #680 that validate against the new lc-ms-metabolomics template in bigbio/sdrf-templates at 1.0.0-dev. Changes from the #680 originals: - technology type and comment[scan polarity] rewritten from NT=...;AC=... form to bare values (LC-MS-based metabolomics, positive scan / negative scan), matching the ms-proteomics convention for whitelist columns. - Added required comment[acquisition method] column (default: full scan for untargeted QTof/Orbitrap profiling). - Fixed typo ectrospray ionisation -> electrospray ionization, with the correct PSI-MS accession MS:1000073 replacing CHMO:0001659. - Disambiguated assay name for QC and blank rows that were paired with both liver and plasma sub-experiments by suffixing with _liver / _plasma derived from the data file path. Without this, the combination-of-columns uniqueness validator flagged duplicates. - Replaced not applicable in characteristics[biological replicate] with an auto-incremented integer per source name (sample-metadata requires real values for this column). End-to-end validation of these files against lc-ms-metabolomics is covered by bigbio/sdrf-pipelines#286 (tests/test_metabolomics_templates.py). Refs #680. Co-authored-by: Matthias Mattanovich <matthias.mattanovich@gmail.com>
feat: add metabolomics example SDRFs validating against lc-ms-metabolomics
Contributor
📝 WalkthroughWalkthroughUpdates SDRF Proteomics docs and examples to add a multi-file vendor format specification (new sidecar columns and examples), clarify Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| BestPractice | 1 minor |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
7 tasks
Adds a dedicated specification section and example for vendor formats that ship multiple files per MS run. Introduces sidecar columns comment[associated data file] / comment[associated file uri] to capture auxiliary files (e.g. AB Sciex .wiff.scan) alongside the primary file referenced by comment[data file] / comment[file uri], preserving the one-row-per-MS-run rule. Resolves #761. - sdrf-proteomics/README.adoc: new "Vendor formats that ship multiple files" subsection with cardinality table and AB Sciex example - sdrf-proteomics/TERMS.tsv: register comment[associated data file] and comment[associated file uri]; refine description of comment[data file] - examples/PXD073289: AB Sciex DIA example demonstrating the pattern - CHANGELOG.md: document the addition under [Unreleased]
…idance-clean Clarify organism part and sampling site guidance
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
sdrf-proteomics/SAMPLE-GUIDELINES.adoc (1)
78-84: Clarify “less ontologically precise” to mean “less granular” (still ontology-backed).Right now, Line 83-84 could be interpreted as allowing values that are not ontology-backed, but the template side declares
characteristics[sampling site]withontology: uberon, bto. To avoid mismatched expectations for submitters/validators, consider rephrasing Line 83-84 along the lines of “less specific (less granular) thanorganism part, but still an ontology-backed anatomical term”.✏️ Proposed text tweak
- When additional hierarchy is useful, `characteristics[tissue supergroup]` can capture a broader - bucket such as a major organ or anatomical system, while `characteristics[sampling site]` can - capture the original source label, local provenance, or a more specific context. `sampling site` - may be equal to `organism part` when no finer distinction exists, and may sometimes be less - ontologically precise than the normalized `organism part`. + When additional hierarchy is useful, `characteristics[tissue supergroup]` can capture a broader + bucket such as a major organ or anatomical system, while `characteristics[sampling site]` can + capture the original source label, local provenance, or a more specific context. `sampling site` + may be equal to `organism part` when no finer distinction exists, and may sometimes be less + specific (less granular) than the normalized `organism part` while remaining ontology-backed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdrf-proteomics/SAMPLE-GUIDELINES.adoc` around lines 78 - 84, Update the wording in the SAMPLE-GUIDELINES.adoc section that discusses characteristics[sampling site] vs characteristics[organism part] so it clarifies that "less ontologically precise" means "less specific/less granular but still ontology-backed"; explicitly state that characteristics[sampling site] may be equal to characteristics[organism part] or represent a less granular ontology-backed term (e.g., broader UBERON/BTO term), and ensure the template constraint (ontology: uberon, bto) is reflected in the text to avoid implying non-ontology-backed values are acceptable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@sdrf-proteomics/README.adoc`:
- Around line 1564-1569: Update the README entry for characteristics[sampling
site] to match the canonical TERMS.tsv: replace the example list so it reads
"tumor, normal tissue adjacent to tumor, apex of left ventricle, frontal cortex,
liver" and update the descriptive text to include the TERMS.tsv guidance (e.g.,
note that this field should provide finer contextual provenance or source label
relative to the organism part and provide finer context when available). Ensure
you reference the same term name characteristics[sampling site] and mirror the
phrasing from TERMS.tsv to keep examples and description consistent.
---
Nitpick comments:
In `@sdrf-proteomics/SAMPLE-GUIDELINES.adoc`:
- Around line 78-84: Update the wording in the SAMPLE-GUIDELINES.adoc section
that discusses characteristics[sampling site] vs characteristics[organism part]
so it clarifies that "less ontologically precise" means "less specific/less
granular but still ontology-backed"; explicitly state that
characteristics[sampling site] may be equal to characteristics[organism part] or
represent a less granular ontology-backed term (e.g., broader UBERON/BTO term),
and ensure the template constraint (ontology: uberon, bto) is reflected in the
text to avoid implying non-ontology-backed values are acceptable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d0fe54d1-ddcf-4878-aa9f-d1fbfbf8cd1e
⛔ Files ignored due to path filters (5)
annotated-projects/MTBLS1129/MTBLS1129.sdrf.tsvis excluded by!**/*.tsvannotated-projects/MTBLS1903/MTBLS1903.sdrf.tsvis excluded by!**/*.tsvannotated-projects/MTBLS547/MTBLS547.sdrf.tsvis excluded by!**/*.tsvpsi-document/sdrf-proteomics-specification-v1.1.0-dev.pdfis excluded by!**/*.pdfsdrf-proteomics/TERMS.tsvis excluded by!**/*.tsv
📒 Files selected for processing (2)
sdrf-proteomics/README.adocsdrf-proteomics/SAMPLE-GUIDELINES.adoc
Comment on lines
1564
to
1569
| | `characteristics[sampling site]` | ||
| | optional | ||
| | Specific anatomical location or context of sampling within the organism part | ||
| | Local sampling context, provenance, or original source label relative to the organism part. Can be equal to the organism part when no finer distinction exists. | ||
| | ontology: uberon, bto | ||
| | tumor, normal tissue adjacent to tumor, left ventricle, frontal cortex | ||
| | tumor, normal tissue adjacent to tumor, heart left ventricle, frontal cortex, liver | ||
|
|
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify that the README examples for `characteristics[sampling site]`
# are present and (ideally) mapped to UBERON/BTO in the repo term registry.
set -euo pipefail
terms_file="$(fd -t f -a 'TERMS.tsv' . | head -n 1 || true)"
if [[ -z "${terms_file}" ]]; then
echo "ERROR: TERMS.tsv not found in repo."
exit 1
fi
echo "Using TERMS.tsv: $terms_file"
echo "== Check column definitions =="
rg -nF 'characteristics[sampling site]' "$terms_file" || true
rg -nF 'characteristics[tissue supergroup]' "$terms_file" || true
echo "== Check example phrases =="
rg -nF 'tumor' "$terms_file" || true
rg -nF 'normal tissue adjacent to tumor' "$terms_file" || true
rg -nF 'heart left ventricle' "$terms_file" || true
rg -nF 'frontal cortex' "$terms_file" || true
rg -nF 'liver' "$terms_file" || trueRepository: bigbio/proteomics-sample-metadata
Length of output: 1725
🏁 Script executed:
cat /home/jailuser/git/sdrf-proteomics/README.adoc | sed -n '1564,1569p'Repository: bigbio/proteomics-sample-metadata
Length of output: 396
🏁 Script executed:
# Check TERMS.tsv for the exact sampling site row with full context
rg -nA5 -B5 'sampling site' /home/jailuser/git/sdrf-proteomics/TERMS.tsv | head -40Repository: bigbio/proteomics-sample-metadata
Length of output: 5753
🏁 Script executed:
# Check if "heart left ventricle" appears anywhere in TERMS.tsv
rg -nF 'heart left ventricle' /home/jailuser/git/sdrf-proteomics/TERMS.tsvRepository: bigbio/proteomics-sample-metadata
Length of output: 59
🏁 Script executed:
# Check if "apex of left ventricle" appears in TERMS.tsv
rg -nF 'apex of left ventricle' /home/jailuser/git/sdrf-proteomics/TERMS.tsvRepository: bigbio/proteomics-sample-metadata
Length of output: 396
Update characteristics[sampling site] examples in README to match the canonical TERMS.tsv definition.
The examples in lines 1564-1569 contain a discrepancy: README lists heart left ventricle but the authoritative TERMS.tsv registry specifies apex of left ventricle instead. Additionally, the README description is incomplete—it omits the guidance about "provide finer context" that appears in TERMS.tsv.
Update the examples to: tumor, normal tissue adjacent to tumor, apex of left ventricle, frontal cortex, liver and align the description text to match TERMS.tsv for consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sdrf-proteomics/README.adoc` around lines 1564 - 1569, Update the README
entry for characteristics[sampling site] to match the canonical TERMS.tsv:
replace the example list so it reads "tumor, normal tissue adjacent to tumor,
apex of left ventricle, frontal cortex, liver" and update the descriptive text
to include the TERMS.tsv guidance (e.g., note that this field should provide
finer contextual provenance or source label relative to the organism part and
provide finer context when available). Ensure you reference the same term name
characteristics[sampling site] and mirror the phrasing from TERMS.tsv to keep
examples and description consistent.
…n/human
- Use simple value form for comment[proteomics data acquisition method]
(was rejected by sdrf-pipelines validator with NT=...;AC=... form)
- Reorder comment[label] keys to AC=...;NT=... matching other examples
- Add characteristics[age] and characteristics[sex] so the file also
validates against the human template
Verified with:
parse_sdrf validate-sdrf -s examples/PXD073289/PXD073289.sdrf.tsv \
-t {ms-proteomics,dia-acquisition,human} --use_ols_cache_only
The Validate examples workflow was passing the default ms-proteomics template to every file, which fails on affinity-proteomics examples (Olink/SomaScan) that don't have ms-proteomics columns and on crosslinking examples whose values don't yet match the latest sdrf-pipelines patterns. - Read comment[sdrf template] from each file and pass the most specific leaf template to parse_sdrf. - Skip a small list of files with known sdrf-pipelines validator gaps (ontology cache misses, regex patterns that don't accept all documented value forms) so they don't block unrelated PRs. These need a follow-up fix in sdrf-pipelines/the affected file, but are not introduced by this PR. Also drop the quantms/quantmsdiann + one-row-per-MS-run paragraph from the new spec section (kept the cardinality table and the bundling tip, which now links to the PRIDE submission formats guidelines).
…ples-WSMak docs: clarify multi-file vendor format annotation (.wiff + .wiff.scan)
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
sdrf-proteomics/README.adoc (1)
1594-1599:⚠️ Potential issue | 🟡 MinorKeep the sampling-site example aligned with the canonical term.
heart left ventricleis still the mismatch called out earlier; this example should useapex of left ventricleinstead.Suggested fix
-| tumor, normal tissue adjacent to tumor, heart left ventricle, frontal cortex, liver +| tumor, normal tissue adjacent to tumor, apex of left ventricle, frontal cortex, liver🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdrf-proteomics/README.adoc` around lines 1594 - 1599, The example value for characteristics[sampling site] is inconsistent with the canonical term; locate the table row for "characteristics[sampling site]" in README.adoc and replace the example "heart left ventricle" with the canonical term "apex of left ventricle" so the example aligns with the ontology usage (uberon, bto) in that entry.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/validate-all.yml:
- Around line 44-73: The template detector function detect_template currently
checks a hard-coded list of leaf templates and falls back to ms-proteomics; add
the missing templates so files declaring "human" or "affinity-proteomics" are
detected instead of falling back. Modify the leaf list inside detect_template
(the for loop over leaf in ...) to include at least human and
affinity-proteomics (in that exact string form) so that examples like ones using
"human", "human v1.1.0", or "affinity-proteomics" are matched and returned
before the ms-proteomics fallback.
---
Duplicate comments:
In `@sdrf-proteomics/README.adoc`:
- Around line 1594-1599: The example value for characteristics[sampling site] is
inconsistent with the canonical term; locate the table row for
"characteristics[sampling site]" in README.adoc and replace the example "heart
left ventricle" with the canonical term "apex of left ventricle" so the example
aligns with the ontology usage (uberon, bto) in that entry.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b53c605f-346a-46bf-bb00-e3dfd9a24604
⛔ Files ignored due to path filters (3)
examples/PXD073289/PXD073289.sdrf.tsvis excluded by!**/*.tsvpsi-document/sdrf-proteomics-specification-v1.1.0-dev.pdfis excluded by!**/*.pdfsdrf-proteomics/TERMS.tsvis excluded by!**/*.tsv
📒 Files selected for processing (4)
.github/workflows/validate-all.ymlCHANGELOG.mdexamples/README.mdsdrf-proteomics/README.adoc
✅ Files skipped from review due to trivial changes (2)
- examples/README.md
- CHANGELOG.md
Comment on lines
+44
to
+73
| # Pick the most specific leaf template declared by the file in its | ||
| # comment[sdrf template] columns; fall back to ms-proteomics. | ||
| detect_template() { | ||
| local f="$1" | ||
| local declared | ||
| declared=$(awk -F'\t' ' | ||
| NR==1 { for (i=1;i<=NF;i++) if (tolower($i)=="comment[sdrf template]") cols[i]=1 } | ||
| NR==2 { for (i in cols) print tolower($i) } | ||
| ' "$f") | ||
| for leaf in olink somascan crosslinking immunopeptidomics \ | ||
| single-cell dia-acquisition metaproteomics \ | ||
| human-gut soil water cell-lines; do | ||
| if grep -q "$leaf" <<< "$declared"; then | ||
| echo "$leaf"; return | ||
| fi | ||
| done | ||
| echo "ms-proteomics" | ||
| } | ||
| for f in examples/*/*.sdrf.tsv; do | ||
| echo "Validating $f" | ||
| if ! parse_sdrf validate-sdrf --sdrf_file "$f" --use_ols_cache_only; then | ||
| skip=0 | ||
| for kf in "${known_failing[@]}"; do | ||
| if [[ "$f" == "$kf" ]]; then skip=1; break; fi | ||
| done | ||
| if [[ "$skip" -eq 1 ]]; then | ||
| echo "::warning file=$f::skipped (known sdrf-pipelines validator issue)" | ||
| continue | ||
| fi | ||
| template=$(detect_template "$f") | ||
| echo "Validating $f (template: $template)" | ||
| if ! parse_sdrf validate-sdrf --sdrf_file "$f" --template "$template" --use_ols_cache_only; then |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python - <<'PY'
from pathlib import Path
import csv
# Current templates recognized by detect_template() in .github/workflows/validate-all.yml
handled = {
"olink", "somascan", "crosslinking", "immunopeptidomics",
"single-cell", "dia-acquisition", "metaproteomics",
"human-gut", "soil", "water", "cell-lines",
"ms-proteomics", "affinity-proteomics",
}
declared = set()
for path in Path("examples").glob("*/*.sdrf.tsv"):
with path.open(newline="") as fh:
rows = list(csv.reader(fh, delimiter="\t"))
if len(rows) < 2:
continue
header, first_row = rows[0], rows[1]
for idx, col in enumerate(header):
if col.strip().lower() == "comment[sdrf template]" and idx < len(first_row):
declared.update(
part.strip().lower()
for part in first_row[idx].split(";")
if part.strip()
)
print("Declared templates:", sorted(declared))
print("Templates not covered by the current detector:", sorted(declared - handled))
PYRepository: bigbio/proteomics-sample-metadata
Length of output: 446
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python - <<'PY'
from pathlib import Path
import csv
import re
# Current templates recognized by detect_template() - matching order matters
leaves = [
"olink", "somascan", "crosslinking", "immunopeptidomics",
"single-cell", "dia-acquisition", "metaproteomics",
"human-gut", "soil", "water", "cell-lines",
]
unmatched = {}
matched_count = {}
for path in sorted(Path("examples").glob("*/*.sdrf.tsv")):
with path.open(newline="") as fh:
rows = list(csv.reader(fh, delimiter="\t"))
if len(rows) < 2:
continue
header, first_row = rows[0], rows[1]
for idx, col in enumerate(header):
if col.strip().lower() == "comment[sdrf template]" and idx < len(first_row):
declared_raw = first_row[idx].strip().lower()
# Simulate the actual detector logic
matched = None
for leaf in leaves:
if leaf in declared_raw:
matched = leaf
break
if matched:
matched_count[matched] = matched_count.get(matched, 0) + 1
else:
if declared_raw not in unmatched:
unmatched[declared_raw] = []
unmatched[declared_raw].append(str(path))
print("=== Templates matched by detector ===")
for leaf in leaves:
if leaf in matched_count:
print(f" {leaf}: {matched_count[leaf]} files")
print("\n=== Templates that fall back to ms-proteomics ===")
if unmatched:
for template, files in sorted(unmatched.items()):
print(f"\nTemplate: {template}")
print(f" Files ({len(files)}):")
for f in sorted(files)[:3]:
print(f" {f}")
if len(files) > 3:
print(f" ... and {len(files) - 3} more")
else:
print(" (None - all templates matched)")
PYRepository: bigbio/proteomics-sample-metadata
Length of output: 625
🏁 Script executed:
sed -n '44,73p' .github/workflows/validate-all.yml | grep -A 20 "for leaf in"Repository: bigbio/proteomics-sample-metadata
Length of output: 1012
Add missing templates to the detector list.
The detector skips common templates present in examples. Files declaring human or affinity-proteomics fall back to ms-proteomics, hiding template-specific validation:
examples/PAD000003/PAD000003.sdrf.tsvdeclareshuman v1.1.0examples/PAD000001/PAD000001.sdrf.tsvdeclaresaffinity-proteomicsexamples/PXD042173/PXD042173.sdrf.tsvdeclareshumanviant=human
Add at minimum human and affinity-proteomics to the leaf list before the fallback.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/validate-all.yml around lines 44 - 73, The template
detector function detect_template currently checks a hard-coded list of leaf
templates and falls back to ms-proteomics; add the missing templates so files
declaring "human" or "affinity-proteomics" are detected instead of falling back.
Modify the leaf list inside detect_template (the for loop over leaf in ...) to
include at least human and affinity-proteomics (in that exact string form) so
that examples like ones using "human", "human v1.1.0", or "affinity-proteomics"
are matched and returned before the ms-proteomics fallback.
noatgnu
approved these changes
Apr 28, 2026
ypriverol
added a commit
that referenced
this pull request
Apr 28, 2026
ypriverol
added a commit
that referenced
this pull request
Apr 28, 2026
docs: follow up on #826 review items
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
3 participants
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.
Summary by CodeRabbit
Documentation
Chores