fedorov / MultiVolumeImporter

A 3D Slicer module to support import of multi-volume data from non-DICOM sources, and provide plugin for importing such data from images saved in DICOM.
http://wiki.slicer.org/slicerWiki/index.php/Documentation/Nightly/Modules/MultiVolumeImporter
Other
9 stars 25 forks source link

Fixed local variable 'sec' referenced before assignment error #23

Closed lassoan closed 7 years ago

lassoan commented 7 years ago

When a file cannot be loaded (e.g., removed from disk) then multivolume importer plugin failed with this error:

[DEBUG][Qt] 02.10.2017 23:22:13 [] (unknown:0) - Could not load  "C:/Users/msliv/Downloads/dicom4miccai/DICOM4MICCAI-Data/QIN-HEADNECK-01-0024-PET/1.2.276.0.7230010.3.1.4.1334872705.15988.1505074246.702.dcm" 
DCMTK says:  No such file or directory
[DEBUG][Qt] 02.10.2017 23:22:13 [] (unknown:0) - SQL failed
 Bad SQL: INSERT OR REPLACE INTO TagCache VALUES(?,?,?)
[DEBUG][Qt] 02.10.2017 23:22:13 [] (unknown:0) - Error text: attempt to write a readonly database Unable to fetch row
[CRITICAL][Stream] 02.10.2017 23:22:13 [] (unknown:0) - Traceback (most recent call last):
[CRITICAL][Stream] 02.10.2017 23:22:13 [] (unknown:0) -   File "C:\D\S4R\Slicer-build\lib\Slicer-4.7\qt-scripted-modules\DICOMLib\DICOMWidgets.py", line 718, in getLoadablesFromFileLists
[CRITICAL][Stream] 02.10.2017 23:22:13 [] (unknown:0) -     loadablesByPlugin[plugin] = plugin.examine(fileLists)
[CRITICAL][Stream] 02.10.2017 23:22:13 [] (unknown:0) -   File "C:/D/S4R/Slicer-build/lib/Slicer-4.7/qt-scripted-modules/MultiVolumeImporterPlugin.py", line 80, in examine
[CRITICAL][Stream] 02.10.2017 23:22:13 [] (unknown:0) -     loadables += self.examineFiles(files)
[CRITICAL][Stream] 02.10.2017 23:22:13 [] (unknown:0) -   File "C:/D/S4R/Slicer-build/lib/Slicer-4.7/qt-scripted-modules/MultiVolumeImporterPlugin.py", line 430, in examineFiles
[CRITICAL][Stream] 02.10.2017 23:22:13 [] (unknown:0) -     mvNodes = self.initMultiVolumes(subseriesLists[key])
[CRITICAL][Stream] 02.10.2017 23:22:13 [] (unknown:0) -   File "C:/D/S4R/Slicer-build/lib/Slicer-4.7/qt-scripted-modules/MultiVolumeImporterPlugin.py", line 646, in initMultiVolumes
[CRITICAL][Stream] 02.10.2017 23:22:13 [] (unknown:0) -     tagValue = self.tm2ms(tagValueStr) # convert to ms
[CRITICAL][Stream] 02.10.2017 23:22:13 [] (unknown:0) -   File "C:/D/S4R/Slicer-build/lib/Slicer-4.7/qt-scripted-modules/MultiVolumeImporterPlugin.py", line 615, in tm2ms
[CRITICAL][Stream] 02.10.2017 23:22:13 [] (unknown:0) -     sec = sec+ssfrac
[CRITICAL][Stream] 02.10.2017 23:22:13 [] (unknown:0) - UnboundLocalError: local variable 'sec' referenced before assignment
[CRITICAL][Stream] 02.10.2017 23:22:13 [] (unknown:0) - Warning: Plugin failed: MultiVolumeImporterPlugin
[CRITICAL][Stream] 02.10.2017 23:22:13 [] (unknown:0) -
[CRITICAL][Stream] 02.10.2017 23:22:13 [] (unknown:0) - See python console for error message.
[INFO][Stream] 02.10.2017 23:22:23 [] (unknown:0) - DICOM Plugin failed: local variable 'sec' referenced before assignment
fedorov commented 7 years ago

xref: https://github.com/commontk/CTK/issues/749

I guess this will be irrelevant once https://github.com/commontk/CTK/pull/750 is merged?

@lassoan let me know if you still want to merge it.

jcfr commented 7 years ago

For reference, as of Slicer r26416, commontk/CTK#750 has been integrated.

lassoan commented 7 years ago

It should not be necessary to merge it. Let's confirm that by testing the tomorrow's nightly build.

fedorov commented 7 years ago

@lassoan sounds good. Although, I think there may be some more deep underlying issue, which in my experience is not reproducible. I experienced this error several times (over months of time) with QuantitativeReporting, and was always unsuccessful trying to reproduce it after restarting Slicer.

lassoan commented 7 years ago

There have been some fixes in DICOM database connection handling in Slicer, so this problem might have been fixed. I got the error when tried to examine images that were still in the database but their files were removed from disk.

fedorov commented 7 years ago

I see - glad there is an explanation for it in your case!

lassoan commented 7 years ago

This is not needed anymore.