Skip to content

fix: Standardize BaseSequence to separate metadata extraction and ASL context generation#43

Open
Devguru-codes wants to merge 1 commit intoOSIPI:mainfrom
Devguru-codes:issue9_arch_fix
Open

fix: Standardize BaseSequence to separate metadata extraction and ASL context generation#43
Devguru-codes wants to merge 1 commit intoOSIPI:mainfrom
Devguru-codes:issue9_arch_fix

Conversation

@Devguru-codes
Copy link

Fixes #42 where the DICOM processing endpoint crashes with ValueError: not enough values to unpack when processing Siemens DICOMs.

The root cause was inconsistent sequence implementation: reports.py expects get_bids_metadata to return a tuple (metadata, asl_context). GE sequences incorrectly returned this tuple from extract_bids_metadata, while Siemens correctly returned only a dict, causing the unpack crash.

Note: This architectural fix implements the exact suggestion provided by mentor Jan Petr when I originally reported this issue in the previous repository before it was included in OSIPI.

Fix

  • Added abstract generate_asl_context method to BaseSequence
  • Modified GE sequences to return only dict from extract_bids_metadata(), moving context generation to generate_asl_context()
  • Added generate_asl_context() to Siemens sequences
  • Updated get_bids_metadata() in main.py to call both methods and return the tuple

Files Changed

  • package/src/pyaslreport/main.py
  • package/src/pyaslreport/sequences/base_sequence.py
  • package/src/pyaslreport/sequences/ge/asl/ge_basic_single_pld.py
  • package/src/pyaslreport/sequences/ge/asl/ge_easl_multi_pld.py
  • package/src/pyaslreport/sequences/siemens/asl/siemens_basic_single_pld.py

API Endpoint Response Comparison

main branch (before fix)

POST /api/report/process/dicom (Siemens DICOM upload)
Status: 500 Internal Server Error
Detail: "ValueError: not enough values to unpack (expected 2, got 1)"
>> VERDICT: Bug is PRESENT.

issue9_arch_fix branch (after fix)

POST /api/report/process/dicom (Siemens DICOM upload)
Status: 200 OK (metadata and asl_context returned as tuple)
>> VERDICT: Bug is FIXED.

Python Test Suite Results

main branch (before fix)

# Test Status Detail
T1 GE extract_bids_metadata returns tuple PASS tuple (wrong architecture)
T2 Siemens extract_bids_metadata returns dict PASS dict
T3 get_bids_metadata for GE returns tuple PASS
T4 get_bids_metadata for Siemens returns tuple FAIL CRASH: too many values to unpack

Summary: PASSED=3 FAILED=1

issue9_arch_fix branch (after fix)

# Test Status Detail
T1 GE extract_bids_metadata returns dict (new arch) PASS dict
T2 Siemens extract_bids_metadata returns dict PASS dict
T3 get_bids_metadata for GE returns tuple (metadata, ctx) PASS tuple
T4 get_bids_metadata for Siemens returns tuple without crashing PASS Success

Summary: PASSED=4 FAILED=0

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.

DICOM endpoint crashes with ValueError: not enough values to unpack due to inconsistent extract_bids_metadata signatures

1 participant