DICOM: be more lenient with slice timing vector in Siemens mosaics

[jdtournier]

There are examples now of Siemens mosaics where the number of entries in the CSA NumberOfImagesInMosaic entry reports a large size than the number of slices - leading to the whole entry being ignored - yet the last few entries are empty. This change allows for this situation, by taking as many entries as there are slices, and ignoring the last few entries.

[bjeurissen]

Looks good! Thanks for fixing this! Can't approve because the mrpad test fails, but this appears unrelated to the pull request..

[thijsdhollander]

Seemed to be only one of the Travis CI jobs that failed (not sure if that's mrpad specific..?). Well, in any case, I restarted the job; let's see what happens.

[Lestropie]

When the number of entries is greater than the number of slices, are the additional entries zero-filled? Is the number of excess entries somehow related to the null space in the mosaic storage? This change will accept such data, but it would also conceivably accept invalid or corrupt data if such could be generated. Depends on if you want to go to the effort of better sanity checking.

[jdtournier]

When the number of entries is greater than the number of slices, are the additional entries zero-filled?

Not even. They're literally empty, as far as I can tell...

Is the number of excess entries somehow related to the null space in the mosaic storage?

Assuming I've understood this right, not even. In the example we have, the CSA entry reports 78 entries when there are 75 slices. The last 3 elements in the list are empty. But I'd expect the mosaic to be 9×9=81, right?

This change will accept such data, but it would also conceivably accept invalid or corrupt data if such could be generated. Depends on if you want to go to the effort of better sanity checking.

Yes, we could sanity-check this more thoroughly, but I'm not sure it's a worthwhile investment. Unfortunately we don't know what assumptions we can rely on regarding this stuff given how proprietary it is, but I also expect Siemens to eventually move away from it in favour of multi-frame (hopefully...?)

[thijsdhollander]

Looks good! Thanks for fixing this! Can't approve because the mrpad test fails, but this appears unrelated to the pull request..

Seemed to be only one of the Travis CI jobs that failed (not sure if that's mrpad specific..?). Well, in any case, I restarted the job; let's see what happens.

I didn't look decently into the logs when I said this; something else seems to indeed be going on... but my brain doesn't seem to be awake enough to deal with it. (a convoluted way of saying "I'm confused")
For some reason some mrpad tests keep failing. It doesn't seem to be the kind of mistake we sometimes ran into in the past (related to not properly branching the test repo and such)... I'm probably overlooking some kind of trivial explanation, but after staring at it for a bit, I'm not getting enlightened. Anyone else got an explanation? The same happens for #1498 ... which is mildly annoying, as I'm trying to get it on master so some other people don't lose their sanity. 😬

[jdtournier]

OK, I cleaned this up by re-committing the relevant changes without changing the test_data commit... Requires a forced-pushed, but hopefully it'll make for a cleaner history...

[jdtournier]

In case anyone's wondering: remaining issues relate to the new version of pylint. Fixes are in #1504. I'll wait until I get confirmation that they're OK from @Lestropie, and once merged to master, we can bring these changes into this PR (and #1498).

[Lestropie]

Seems to be the only option that will be compatible with "valid" data; more stringent checks are not possible.

[thijsdhollander]

Yes, I agree.