Compatible with pydicom v3

[howff]

Description

Related issues: #266

Finishing off PR #268

Checklist

• I have commented my code, particularly in hard-to-understand areas
• My changes generate no new warnings
• My code follows the style guidelines of this project

Open questions

Questions that require more discussion or to be addressed in future development:

[vsoch]

This looks great! One design suggestion that I think will make this hugely easier - we can add a function to utils that basically returns the pydicom read_file / dcmread function, and that way if/when the way to read dicom changes (it has changed multiple times over the years) we can change just one spot. What do you think?

[howff]

Just a couple of thoughts:

1. I don't think this PR needs to wait for any changes to pydicom, it's a simple change and without it deployment is severely hindered
2. In general I agree with having some abstraction to hide details which might change but in this case I think it's better if pydicom can not keep changing its API now that it's at v3, is mature, and is heavily used

[vsoch]

I don't think this PR needs to wait for any changes to pydicom, it's a simple change and without it deployment is severely hindered

I am not saying changes to pydicom, I'm saying that in some utils.py you'd have:

def dcmread(filename)
    return pydicom.dcrmread(filename)

And then across the code base:

import deid.utils as utils

dcm = utils.dcmread(filename)

And then if there is a change to pydicom (which there has been several times already for this specific function) we are changing one place instead of 200. Does that make sense?

[howff]

As primary owner/maintainer I will let you decide on changes such as that. I just wanted to get the fix for v3 across the line!

[vsoch]

@howff I think that would be the better design. Would you have time to work on this?

[howff]

I would prefer to keep this PR for a bug fix and use a different PR for a feature request, but since you asked so nicely and deid really needs to be fixed asap I've made the requested changes.

[vsoch]

Thank you!

[vsoch]

Please add this as a function to the utils.py file:

https://github.com/pydicom/deid/blob/1a0ef4480bf958966f1fd9d153247664dd0fc169/deid/dicom/utils.py

Then for use you can do:

import deid.dicom.utils as utils
dcm = utils.dcmread(filename)

[howff]

It didn't work, circular reference

[vsoch]

What if we did:

utils/
__init__.py 
fileio.py
dcmread.py

To avoid the circular reference? The basic idea is around organization - we want dcmread to clearly be in a utils module.

[vsoch]

As a maintainer I review for functionality and design, with an eye for looking forward. I gave you my feedback and design suggestions, which did not have any bug. The only problem here is this comment. Please try to follow the code of conduct moving forward.

[howff]

I've done everything which you asked me to do, but this PR has been closed without being merged. Can you confirm that you've done everything which this PR did, but in your own PR, so that I know whether to delete this branch?

[howff]

@vsoch Please excuse my frustration. The fix only required

• deleting two space characters
• deleting ,<2

but I spent two days doing the extra work (and my wife complained because I was still working on it after 11pm). I am glad that you managed to achieve everything yourself in about 20 minutes.

[vsoch]

For transparency, this is the comment I responded to above (that you deleted).

I was respectful in my interactions, kind, and always asked for your thoughts and if you had time to work on it. I'm sorry for your frustration - I hope you find strategies to respond to review that don't lead to that in the future.