tomaroberts / nii2dcm

nii2dcm: NIfTI to DICOM creation with Python
Other
53 stars 13 forks source link

dunamai RuntimeError and UnboundLocalError: local variable 'nZ' referenced before assignment #30

Open astewartau opened 6 months ago

astewartau commented 6 months ago

Thanks for your work on nii2dcm! I'm considering integrating this as part of my QSM processing toolbox, QSMxT.

However, I'm having problems using it in my miniconda setup:

(qsmxt) ~/.../data/qsmtest/qsm: nii2dcm sub-1_ses-20231020_part-phase_T2Starw_romeo-unwrapped_normalized_pdf_rts_twopass_ref.nii dicom-output-directory/ --dicom-type MR
Traceback (most recent call last):
  File "/home/ashley/neurodesktop-storage/miniconda3/envs/qsmxt/bin/nii2dcm", line 5, in <module>
    from nii2dcm.__main__ import cli
  File "/home/ashley/neurodesktop-storage/miniconda3/envs/qsmxt/lib/python3.8/site-packages/nii2dcm/__main__.py", line 9, in <module>
    from nii2dcm._version import __version__
  File "/home/ashley/neurodesktop-storage/miniconda3/envs/qsmxt/lib/python3.8/site-packages/nii2dcm/_version.py", line 2, in <module>
    __version__ = Version.from_git().serialize(metadata=False, style=Style.SemVer)
  File "/home/ashley/neurodesktop-storage/miniconda3/envs/qsmxt/lib/python3.8/site-packages/dunamai/__init__.py", line 1058, in from_git
    _detect_vcs(vcs)
  File "/home/ashley/neurodesktop-storage/miniconda3/envs/qsmxt/lib/python3.8/site-packages/dunamai/__init__.py", line 355, in _detect_vcs
    raise RuntimeError(
RuntimeError: This does not appear to be a Git project

It is related somehow to the dunamai package. Any idea what could be causing this?

I tried initialising a git repository in the local directory to see if that could be a workaround, but then I have a new problem:

(qsmxt) ~/.../data/qsmtest/qsm: nii2dcm -d MR sub-1_ses-20231020_part-phase_T2Starw_romeo-unwrapped_normalized_pdf_rts_twopass_ref.nii dicom-output-directory/
Traceback (most recent call last):
  File "/home/ashley/neurodesktop-storage/miniconda3/envs/qsmxt/bin/nii2dcm", line 8, in <module>
    sys.exit(cli())
  File "/home/ashley/neurodesktop-storage/miniconda3/envs/qsmxt/lib/python3.8/site-packages/nii2dcm/__main__.py", line 55, in cli
    run_nii2dcm(
  File "/home/ashley/neurodesktop-storage/miniconda3/envs/qsmxt/lib/python3.8/site-packages/nii2dcm/run.py", line 38, in run_nii2dcm
    nii2dcm_parameters = nii2dcm.nii.Nifti.get_nii2dcm_parameters(nii)
  File "/home/ashley/neurodesktop-storage/miniconda3/envs/qsmxt/lib/python3.8/site-packages/nii2dcm/nii.py", line 38, in get_nii2dcm_parameters
    nInstances = nZ*nF
UnboundLocalError: local variable 'nZ' referenced before assignment

Looking at your code in nii.py, I can see that this is happening because of the dimensions of my file:

>>> nii.header['dim']
array([  3, 164, 205, 205,   0,   0,   0,   0], dtype=int16)

Which will not be handled correctly by the initialization code:

https://github.com/tomaroberts/nii2dcm/blob/09d20e02f61da2aa5e7a2e57bd420828021f0d92/nii2dcm/nii.py#L29C1-L35C58

My file is a 3D volume with 205 slices, each with dimensions 164x205. It is a quantitative susceptibility map with floating-point values in the range of about -4 to +3, with most values around zero.

astewartau commented 6 months ago

I've implemented a quick and crude solution for this in my fork to work only for my QSM images:

https://github.com/astewartau/nii2dcm/commit/ccd2ba516212711844aed956ecda41feccc18b2c

I also solved another problem specific to my data - QSM images in NIfTI pipelines are floating-point values centred around zero, though they can have outliers in either direction. DICOMs for QSM images are usually centred around 2048, so a robust conversion is required, which I've tried to do in my updated run.py.

astewartau commented 6 months ago

I also added a command-line option to enable this. :)

https://github.com/astewartau/nii2dcm/commit/b54fd20170a31f05fcdd24690f6576eddcf7e077

If you think this would be useful, I can create a PR or you could let me know if there's a better way?

tomaroberts commented 6 months ago

@astewartau – thanks for submitting the Issue! I've got a very busy week, but will look into this soon.

On the miniconda part – I tend to use pip with my virtual environments, so I haven't tested it extensively with other package managers. This is not a suggestion as a solution, but as a sanity check, have you tried installing nii2cm into a fresh venv via pip following the README instructions?

Thanks for the code changes – I'll look at them properly and then get back to you about integration/PR.

astewartau commented 6 months ago

Hi @tomaroberts, thanks for offering to look into this! I don't get the git issue when I use a virtual environment - it seems to only occur in my Miniconda setup.

Just a little more information that might be helpful - the susceptibility map uses float data, which I've learned we can store in DICOM if we use the FloatPixelData part of the pydicom Dataset object rather than PixelData, which would mean we don't need to force/scale the data to fit uint16 like I've attempted. However, I'm having issues creating a dummy DICOM file with FloatPixelData via pydicom. This might be relevant:

https://github.com/pydicom/pydicom/issues/1077

tomaroberts commented 6 months ago

@astewartau

I'm not very familiar with FloatPixelData, but you might want to also look up the ImagePixel module in the DICOMs (e.g. https://github.com/tomaroberts/nii2dcm/blob/main/nii2dcm/modules/image_pixel.py), because these influence contrast. You approach to scaling to uint16 is not a bad idea to pursue.

I see you've forked the repo. My intention was to have classes for different modalities or subtypes. Your QSM work would align with this.

I haven't fleshed it out fully yet, but you can create Classes which inherit the MRI DICOM modules, such as I have done for SVR: https://github.com/tomaroberts/nii2dcm/blob/main/nii2dcm/svr.py

You could make an equivalent, let's call it qsm.py or whatever you fancy, and then use that to override the default DicomMRI tags in your QMS DICOM. e.g. this might be a way of testing out the windowing.

You may also need to edit the command line code so you can supply QSM as a -d / --dicom-type option, e.g. --dicomtype QSM.

jcohen02 commented 6 months ago

Just a quick addition here to link this with #31 - the first issue you describe at the top of your issue, @astewartau, regarding dunamai looks like it can be addressed by the workarounds in #31 until a permanent fix is put in place. However, it looks like initialising the git repo probably also worked around this 🙂.

The second part where you highlight the UnboundLocalError: local variable 'nZ' referenced before assignment error looks to be something separate. Great to see that you were able to implement a solution to this - I'll leave this issue with you and @tomaroberts but just wanted to flag #31.

tomaroberts commented 5 months ago

@astewartau – I've released v0.1.5 of nii2dcm, which fixes the pip install issue, if you want to try it out.

If you are still interested in the integration with your QSM toolbox, let me know and I'll re-read this issue more thoroughly, as it's been a little while.

Thanks.

astewartau commented 3 months ago

Thanks so much! That seems to be working well.

I may have some time to come back to the scaling issue for floating-point maps such as QSM soon, but for now I've also created a pull request for the undefined variable nZ issue. This issue occurs with valid 3D NIfTI images that store their dimensions slightly differently than assumed (see original post).

https://github.com/tomaroberts/nii2dcm/pull/39

Hopefully the fix makes sense! :)

tomaroberts commented 3 months ago

Great! Thanks for PR. Will take a look :)