MRD Upgrade tool

[naegelejd]

Introduces a new Python tool to upgrade MRD files to the current version of MRD.

Initially, it supports the upgrade path from v2.2.0 (previous) to v2.2.1 (current).

[Copilot]

Pull request overview

Introduces a Python tool (mrd-upgrade) plus library API to upgrade existing MRD binary files from older schema versions (initially v2.2.0) to the current schema (v2.2.1). Also refactors how the MRD version string is propagated through builds (reading the top-level VERSION file directly instead of relying on the MRD_VERSION_STRING env var) and includes an unrelated refactor of mrd_to_ismrmrd.py.

Changes:

• New upgrade tool: python/mrd/tools/{upgrade.py,_schema_registry.py,_v220_reader.py} with CLI entry point, version detection, v2.2.0 read-only serializers, and a chained upgrade dispatcher.
• End-to-end test harness under test/upgrade/ (shell driver + v2.2.0 generator + verifier) and a just upgrade-test recipe.
• Version-string plumbing reworked: setup.py, conda recipes, CMake, Matlab, justfile, Dockerfile, devcontainer and CI now derive the version from the VERSION file / _version.py; the MRD_VERSION_STRING env var is removed.

Reviewed changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
UPGRADE.md	Design/rationale document for the upgrade tool.
docs/python/upgrade.md	User-facing docs for mrd-upgrade.
docs/.vitepress/config.mts	Adds upgrade page to docs sidebar.
python/pyproject.toml	Adds mrd-upgrade script entry; bumps ismrmrd dep.
python/setup.py	Reads version from mrd/_version.py instead of env var.
python/mrd/_version.py	Generated version literal (__version__ = "2.2.1").
python/mrd/tools/upgrade.py	CLI + upgrade_mrd_file public API and chained-step dispatcher.
python/mrd/tools/_schema_registry.py	Embedded v2.2.0 schema literal and identify_file_version.
python/mrd/tools/_v220_reader.py	v2.2.0 read-only serializers producing v2.2.1 objects.
python/mrd/tools/mrd_to_ismrmrd.py	Refactor to use constructor kwargs; unrelated to upgrade tool.
python/conda/meta.yaml, cpp/conda/meta.yaml	Read version from VERSION via load_file_regex.
cpp/CMakeLists.txt	Prefer PKG_VERSION then VERSION file; remove env-var path.
matlab/buildToolbox.m	Read version from VERSION file.
docker/Dockerfile, docker/build-images.sh	Drop MRD_VERSION_STRING build-arg; copy VERSION into image.
justfile	Add upgrade-test; generate _version.py; drop env var.
environment.yml	Add ismrmrd-python>=1.15.0; drop pip ismrmrd==1.14.2.
.readthedocs.yaml, .devcontainer/devcontainer.bashrc, .github/workflows/mrd_build.yml	Remove env-var setup; fetch tags in CI for git archive v2.2.0.
test/upgrade/test-upgrade.sh	End-to-end test driver.
test/upgrade/generate_v220.py	Produces a v2.2.0 stream under the v2.2.0 codecs.
test/upgrade/verify_upgrade.py	Validates the upgraded v2.2.1 file.

Comments suppressed due to low confidence (11)

python/mrd/tools/mrd_to_ismrmrd.py:107

• Behavior change: previously the code only set meas.patientPosition if meas_info.patient_position in pos_map. Now pos_map[meas_info.patient_position] is accessed unconditionally, which will raise KeyError if a PatientPosition enum value is ever added to MRD but not yet mirrored in pos_map. Consider using pos_map.get(...) with a sensible default (as you did for trajectory at line 266) or restore the membership guard.

        meas = ismrmrd.xsd.measurementInformationType(
            patientPosition=pos_map[meas_info.patient_position]
        )

python/mrd/tools/mrd_to_ismrmrd.py:290

• The previous code guarded all four attribute accesses (if par_img.acceleration_factor, plus per-field is not None checks). The new code unconditionally accesses par_img.acceleration_factor.kspace_encoding_step_1 and .kspace_encoding_step_2. If par_img.acceleration_factor is None (which can occur if the MRD producer omitted it even though the schema marks it required), this will raise AttributeError. Also, previously a None kspace_encoding_step_* was skipped; now None is forwarded into accelerationFactorType, which may not accept it.

            acc = ismrmrd.xsd.accelerationFactorType(
                kspace_encoding_step_1=par_img.acceleration_factor.kspace_encoding_step_1,
                kspace_encoding_step_2=par_img.acceleration_factor.kspace_encoding_step_2
            )
            parallel = ismrmrd.xsd.parallelImagingType(accelerationFactor=acc)

python/mrd/tools/mrd_to_ismrmrd.py:205

• limits is now constructed and assigned to enc.encodingLimits unconditionally, even when enc_mrd.encoding_limits is falsy. Previously the field was only set when limits were actually present. This is likely benign but is a behavior change: the resulting ISMRMRD XML will now always contain an (empty) encodingLimits element. Confirm this is intended.

        limits = ismrmrd.xsd.encodingLimitsType()
        if enc_mrd.encoding_limits:

python/mrd/tools/mrd_to_ismrmrd.py:156

• Behavior change: previously coilNumber / coilName were only assigned when non-None. Now they're passed in directly. If label.coil_number is None (the MRD type may permit it depending on producer behavior), the coilLabelType constructor may reject None for an integer field. Consider preserving the previous guarded assignment.

                coil = ismrmrd.xsd.coilLabelType(
                    coilNumber=label.coil_number,
                    coilName=label.coil_name
                )

python/mrd/tools/_schema_registry.py:57

• identify_file_version reads the 4-byte binary format version with stream.read_view(4) but never validates it. BinaryProtocolReader.__init__ does check version != CURRENT_BINARY_FORMAT_VERSION. As a result, a file written with a future incompatible Yardl binary format could still be misidentified by schema string match without raising a clear error. Consider validating the format version here too (or document why it's intentionally skipped).

        _fmt_version = stream.read_view(4)
        schema = string_serializer.read(stream)

python/mrd/tools/upgrade.py:158

• When --in-place is used, the upgraded file is created via tempfile.mkstemp (which creates the file with mode 0600) and then os.replaced over the source. This means an in-place upgrade will silently strip the source file's original permissions (e.g. world-readable scans become user-only). Consider copying the source's st_mode (and possibly st_uid/st_gid if running as root) to the temp file before the rename, or document this side effect.

        # Write to a temp file next to the source, then atomically replace.
        src_dir = os.path.dirname(os.path.abspath(src))
        fd, tmp_path = tempfile.mkstemp(dir=src_dir, suffix=".tmp")
        os.close(fd)
        try:
            upgrade_mrd_file(src, tmp_path)
            os.replace(tmp_path, src)

python/mrd/tools/upgrade.py:169

• When args.output is not provided and --in-place is not used, the destination is computed as src + ".upgraded". If that file already exists it is silently overwritten. Consider checking os.path.exists(dst) and refusing (or requiring --force) to avoid clobbering an existing file.

        dst = args.output if args.output else src + ".upgraded"
        upgrade_mrd_file(src, dst)
        print(f"Upgraded {src!r} → {dst!r}")

test/upgrade/test-upgrade.sh:24

• git archive v2.2.0 python/mrd/ requires the v2.2.0 tag to be present in the local clone. CI fetches with fetch-depth: 0 and fetch-tags: true (added in this PR), but developers running just upgrade-test on a shallow clone, or bash test/upgrade/test-upgrade.sh directly, will get a confusing fatal: not a valid object name error. Consider an upfront check (git rev-parse v2.2.0^{tag}) with a clearer message, or auto-fetching the tag if missing.

# Extract all v2.2.0 files from git.
git -C "$WORKSPACE" archive v2.2.0 python/mrd/ | tar -x --strip-components=2 -C "$V220_MOD"

test/upgrade/test-upgrade.sh:40

• test-upgrade.sh relies on the side effect that git archive ... | tar -x --strip-components=2 -C "$V220_MOD" extracts the v2.2.0 tree into $V220_MOD (which is .../mrd_v220/mrd). Because the v2.2.0 tag predates this PR, the extracted tree won't contain mrd/tools/ — so from mrd.tools._schema_registry import identify_file_version (used in the inline Python snippet on lines 32-40) only works because the python3 interpreter that runs that snippet uses PYTHONPATH=${WORKSPACE}/python:... (line 14), where tools/ exists. This is subtle and easy to break later if PYTHONPATH ordering or the inline snippet's working dir changes. A short comment explaining which PYTHONPATH "wins" for each invocation would make this more robust.

PYTHONPATH="$WORKDIR/mrd_v220:${PYTHONPATH:-}" python3 "$TESTDIR/generate_v220.py" "$V220"

echo "  Verifying source file is detected as v2.2.0 ..."
detected=$(python3 - "$V220" <<'EOF'
import sys
from mrd.tools._schema_registry import identify_file_version
v = identify_file_version(sys.argv[1])
if v != "2.2.0":
    raise RuntimeError(f"Expected 2.2.0, got {v!r}")
print(v)
EOF
)

python/mrd/tools/_schema_registry.py:56

• Linting/static-analysis nit: _fmt_version is read and stored but never used. Either drop the assignment (just stream.read_view(4) for side-effect of advancing the stream) or validate it against CURRENT_BINARY_FORMAT_VERSION from _binary.py. As written, some lint configs will flag this as an unused variable.

        _fmt_version = stream.read_view(4)

test/upgrade/test-upgrade.sh:64

• The error-case test only covers "already-current file". It doesn't cover the other two ValueError paths in upgrade_mrd_file: (a) unrecognised schema (file with magic bytes but unknown schema string) and (b) a supported-version-but-broken-chain failure mode. Given how easy these are to exercise (a single malformed temp file), adding even one assertion for the "unrecognised" branch would catch a meaningful regression.

echo "  Testing error cases ..."
python3 - "$V221" <<'EOF'
import sys
from mrd.tools.upgrade import upgrade_mrd_file
try:
    upgrade_mrd_file(sys.argv[1], sys.argv[1] + ".should_not_exist")
    raise RuntimeError("Expected ValueError for already-current file")
except ValueError:
    pass  # expected
EOF

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.