PR: Update load_dicom to accommodate Pydicom 3.0

[mrclary]

pydicom released version 3.0 three weeks ago, resulting in recent spyder-kernels test failures for spyder_kernels/utils/tests/test_iofuncs.py::test_load_dicom_files.

Unfortunately, this includes breaking changes. In particular, pydicom.dicomio.read_file is renamed pydicom.dicomio.dcmread. Also unfortunate, pydicom does not support Python <3.10. Also unfortunate, this requirement is neither enforced by pip nor conda for some reason, i.e. pydicom 3.0.1 is installed into Python <3.10 environments.

• Update load_dicom to accommodate only two configurations
  • Pydicom >=3.0 and Python >=3.10
  • Pydicom <3.0 and Python <3.10
• Skip test_load_dicom_files if Python <3.10, since CI will install Pydicom >=3

[mrclary]

@ccordoba12, I think that the environment cache will need to be cleared so that the Linux pip test for Python 3.10 will pass. pydicom 3.0.1 is still being installed instead of 2.4.4.

[ccordoba12]

Unfortunately, this includes breaking changes. In particular, pydicom.dicomio.read_file is renamed pydicom.dicomio.dcmread.

Ok, then, please change this

try:
    data = dicomio.read_file(filename, force=True)
except TypeError:
    data = dicomio.read_file(filename)

to be

try:
   # For Pydicom 3/Python 3.10+
    data = dicomio.dcmread(filename, force=True)
except TypeError:
    data = dicomio.dcmread(filename)
except AttributeError:
    # For Pydicom 2/Python 3.9-
    try:
        data = dicomio.read_file(filename, force=True)
    except TypeError:
        data = dicomio.read_file(filename)

That will make that code run for Pydicom greater than and less than 3.0.

Also unfortunate, pydicom does not support Python <3.10. Also unfortunate, this requirement is neither enforced by pip nor conda for some reason, i.e. pydicom 3.0.1 is installed into Python <3.10 environments.

Ok, then please skip the test that checks our Pydicom integration for Python versions less than 3.10. And then you can remove the constraint you added to Pydicom.

It'd be nice too if you could inform Pydicom and Conda-forge maintainers that it's 3.0 version only supports 3.10+. But that's up to you.

[ccordoba12]

Also unfortunate, this requirement is neither enforced by pip nor conda for some reason, i.e. pydicom 3.0.1 is installed into Python <3.10 environments.

The requirement is correctly enforced by pip. Otherwise our tests with pip packages for Python 3.8/3.9 would fail with the last change you did in this PR. So, the problem is only present in Conda-forge.

And in that case, I think the best we can do is to submit a PR to the Pydicom feedstock to fix the situation. If we do that, this shouldn't be necessary:

Ok, then please skip the test that checks our Pydicom integration for Python versions less than 3.10. And then you can remove the constraint you added to Pydicom.

[mrclary]

try:
   # For Pydicom 3/Python 3.10+
    data = dicomio.dcmread(filename, force=True)
except TypeError:
    data = dicomio.dcmread(filename)
except AttributeError:
    # For Pydicom 2/Python 3.9-
    try:
        data = dicomio.read_file(filename, force=True)
    except TypeError:
        data = dicomio.read_file(filename)

That will make that code run for Pydicom greater than and less than 3.0.

Also unfortunate, pydicom does not support Python <3.10. Also unfortunate, this requirement is neither enforced by pip nor conda for some reason, i.e. pydicom 3.0.1 is installed into Python <3.10 environments.

Ok, then please skip the test that checks our Pydicom integration for Python versions less than 3.10. And then you can remove the constraint you added to Pydicom.

If we skip Pydicom integration test for Python <3.10, then there is no need catch the AttributeError.

[ccordoba12]

If we skip Pydicom integration test for Python <3.10, then there is no need catch the AttributeError

This is not only a matter of fixing our tests. It also has to do with maintaining Pydicom support for Python 3.8/3.9 and 3.10+ at the same time. We need to do that because 3.9 will reach end-of-life in a year or so (3.8 will become unsupported at the end of October though).

[mrclary]

If we skip Pydicom integration test for Python <3.10, then there is no need catch the AttributeError

This is not only a matter of fixing our tests. It also has to do with maintaining Pydicom support for Python 3.8/3.9 and 3.10+ at the same time. We need to do that because 3.9 will reach end-of-life in a year or so (3.8 will become unsupported at the end of October though).

Yep, you are correct. I misspoke.

[mrclary]

@ccordoba12, this is ready for review

[mrclary]

Actually, pydicom = 3.0.1_1 was just built per conda-forge/pydicom-feedstock#40. This should correctly enforce <3.0 for Python <3.10, so we may not have to skip the test.

[ccordoba12]

One additional suggestion so test_load_dicom_files works for all our supported Python versions, but only when using pip packages.

[ccordoba12]

Looks good to me now, thanks @mrclary!

[ccordoba12]

@meeseeksdev please backport to 3.x