ImagingDataCommons / highdicom

High-level DICOM abstractions for the Python programming language
https://highdicom.readthedocs.io
MIT License
168 stars 35 forks source link

Improvement of specimen description #153

Closed hackermd closed 2 years ago

hackermd commented 2 years ago

This PR introduces the following changes:

hackermd commented 2 years ago

Very unfamiliar with this part of the standard, but why does the processing_type even need to be passed if it's determined by the type of the processing_procedure?

You are right, it's not really required. I realized that having the two parameters (processing_type and processing_procedure) is error prone and therefore added checks to ensure that the arguments match. We could now deprecate processing_type. However, since it's currently the first parameter, we would break the API even if we would make it optional for the purpose of deprecation (since we would need to move the parameter). Thoughts?

CPBridge commented 2 years ago

Hmm, I'd be tempted to deprecate as gracefully as we can as we are still pre 1.0 but I defer to your judgement

hackermd commented 2 years ago

@CPBridge I deprecated the parameter and added a paragraph to the release notes (f8ddb989aa039c2944b2746eff46f4f84660eae7).