Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ValueError by removing [0] to properly unpack indexed data #2114

Merged

Conversation

joshuacwnewton
Copy link
Contributor

@joshuacwnewton joshuacwnewton commented Apr 19, 2024

Description

This fixes a bug introduced in d87fa5b:

  File "/__w/spinalcordtoolbox/spinalcordtoolbox/spinalcordtoolbox/deepseg/inference.py", line 228, in segment_nnunet
    pred = predictor.predict_single_npy_array(
  File "/__w/spinalcordtoolbox/spinalcordtoolbox/python/envs/venv_sct/lib/python3.9/site-packages/nnunetv2/inference/predict_from_raw_data.py", line 442, in predict_single_npy_array
    dct = next(ppa)
  File "/__w/spinalcordtoolbox/spinalcordtoolbox/python/envs/venv_sct/lib/python3.9/site-packages/batchgenerators/dataloading/data_loader.py", line 126, in __next__
    return self.generate_train_batch()
  File "/__w/spinalcordtoolbox/spinalcordtoolbox/python/envs/venv_sct/lib/python3.9/site-packages/nnunetv2/inference/data_iterators.py", line 193, in generate_train_batch
    image, seg_prev_stage, props, ofname = self._data[idx][0]
ValueError: not enough values to unpack (expected 4, got 1)

By combining the indexes into a single line, I believe the previous commit needed to remove the [0] index in order for the indexed data to properly unpack.

Testing

I tested this PR with our downstream package in spinalcordtoolbox/spinalcordtoolbox#4448 (comment).

The specific command that we run that triggers this issue in the first place can be found here.

(I'm surprised that this wasn't caught by nnunetv2's test suite! I'm not sure if this issue is specific to predict_single_npy_array, but I would presume not? EDIT: Ah, I see now. The tests aren't set up to run on PRs...)

Related issues/PRs

Fixes #2112.

This fixes a bug introduced in MIC-DKFZ@d87fa5b. 

`[0]` refers specifically to `files = self._data[idx][0]`. By combining the indexes into a single link, the previous commit needed to remove the `[0]` index.
@FabianIsensee FabianIsensee self-assigned this Apr 19, 2024
joshuacwnewton added a commit to spinalcordtoolbox/spinalcordtoolbox that referenced this pull request Apr 19, 2024
…_single_npy_array` (#4448)

## Description
<!-- describe what the PR is about. Explain the approach and possible
drawbacks.It's ok to repeat some text from the related issue. -->

I've opened an upstream PR to fix this issue here:
MIC-DKFZ/nnUNet#2114

I'm hoping that this will be merged for nnunetv2==2.4.2, hence avoiding
only 2.4.0/2.4.1.

## Linked issues
<!-- If the PR fixes any issues, indicate it here with issue-closing
keywords: e.g. Resolves #XX, Fixes #XX, Addresses #XX. Note that if you
want multiple issues to be autoclosed on PR merge, you must use the
issue-closing verb before each relevant issue: e.g. Resolves #1,
Resolves #2 -->

Fixes #4444.
@FabianIsensee FabianIsensee merged commit 0223414 into MIC-DKFZ:master Apr 25, 2024
1 check failed
@FabianIsensee
Copy link
Member

Thanks a lot! That one slipped through somehow 😅

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.

v2.4.1 raises ValueError: not enough values to unpack when calling predict_single_npy_array
2 participants