Mypy: Item "Code" of "Union[_CodesDict, _CID_Dict, Code]" has no attribute "XXX"

[hackermd]

When running mypy on the codes provided via pydicom.sr.codedict.codes, I get the following error:

mypy -c 'from pydicom.sr.codedict import codes; codes.SCT.Tissue'
<string>:1: error: Item "Code" of "Union[_CodesDict, _CID_Dict, Code]" has no attribute "Tissue"

Since the attributes are dynamically fetched from the underlying dict via _CodesDict.getattr(), mypy doesn't know about the individual attributes.

In addition, the type annotation Union[_CodesDict, _CID_Dict, Code] causes several other issues in application code. I tend argue that the type of codes.SCT.Tissue should just be Code rather than a complex Union.

Related to #1251.

[scaramallion]

The return type is correct, that method is really dynamic, unfortunately. It's probably a sign that it needs to be refactored.

>>> from pydicom.sr import codes
>>> type(codes)
<class 'pydicom.sr.codedict._CodesDict'>
>>> type(codes.SCT)
<class 'pydicom.sr.codedict._CodesDict'>
>>> type(codes.SCT.Tissue)
<class 'pydicom.sr.coding.Code'>
>>> type(codes.cid2)
<class 'pydicom.sr.codedict._CID_Dict'>

You should cast (or ignore).

from pydicom.sr import codes
from pydicom.sr.codedict import _CodesDict
from typing import cast

code = cast(_CodesDict, codes.SCT).Tissue

[hackermd]

Thanks for your feedback @scaramallion. We are using this abstraction very frequently in highdicom and I don't consider casting or ignoring viable options. The cast is quite ugly and really defeats the purpose of having this nice abstraction in the first place. Ignoring is a) not ideal and b) doesn't solve the problem entirely, because the Union type is viral and affects other calls downstream, where Union[Code, ...] is expected.

I think we should refactor this and generally avoid return values of Union type. Would it make sense to implement a different class for each hierarchical "level" (_CodesDict, _CID_Dict, Code) so that each __getattr__ would return a value of the appropriate type?

[scaramallion]

Would it make sense to implement a different class for each hierarchical "level"

Yeah, I think so. They could probably be "public" classes, too.

[darcymason]

Assigning this to pydicom v3.0, where the plan is to drop Python versions and therefore have access to better typing syntax and types.