Python keyword issue in pydicom.sr.codedict

[seandoyle]

trafficstars

One of the codes in codes.cid3716 can not be accessed as a Python attribute. I would expect all codes to be accessible in a consistent manner.

In a python interpreter - if I enter codes.cid3716.None I expect that the output would be

 Code(value='260413007', scheme_designator='SCT', meaning='None', scheme_version=None)

but instead I get

File "<stdin>", line 1
    codes.cid3716.None
                  ^
SyntaxError: invalid syntax

The values are there - the following works:

>>> getattr(codes.cid3716, 'None')
Code(value='260413007', scheme_designator='SCT', meaning='None', scheme_version=None)

To reproduce:

from pydicom.sr.codedict import codes
none_val = codes.cid3716.None

Version info:

module       | version
------       | -------
platform     | macOS-10.16-x86_64-i386-64bit
Python       | 3.8.6 (default, Oct  8 2020, 14:06:32)  [Clang 12.0.0 (clang-1200.0.32.2)]
pydicom      | 2.0.0
gdcm         | _module not found_
jpeg_ls      | _module not found_
numpy        | 1.19.4
PIL          | 8.0.1

@hackermd's suggestion was to add some logic to generate_concept_dicts.py. Perhaps something like:

import keyword
name_is_keyword = keyword.iskeyword(name)
if name_is_keyword:
    name = f’{name}__’

[darcymason]

I think adding an underscore as suggested is reasonable - certainly x.None is a syntax error in Python for any x, so we have no control over that.

Another possibility is for pydicom to have a workaround in case a name isn't found- before throwing an AttributeError, see if there is a trailing underscore that can be removed and then if that name is found return its value. This solution then only requires time for the very rare keyword case, which I think is acceptable.

However, the @hackermd solution does mean that the name with trailing underscore will be shown by dir() and autocomplete, so perhaps that is a better solution.

Unfortunately, the user will not get very good error info from the SyntaxError, and I don't think that there is anything we can do about that, except have something in the documentation for them to find on a web search.

[darcymason]

...and... are you able to do a PR for the @hackermd solution? ... including a test and a little documentation with the SyntaxError for people to find?

[seandoyle]

Sure.

[hackermd]

trailing underscore will be shown by dir() and autocomplete, so perhaps that is a better solution.

Yes, I was hoping that a trailing underscore will be intuitive to use. There shouldn't be many conflicts of this kind, since there aren't a lot of reserved symbols starting with a capital letter (None, Exception, ...).

[darcymason]

Looking this over again, I'd propose this combination of solutions discussed:

• leave the keyword name in the dictionary without a trailing underscore. This to me is justified because of the getattr example where the text is fine when quoted. It is only the 'convenience' form with the dotted notation that causes a problem.
• update __dir__ to add trailing "_" to any items that are a keyword, which can cue the user to use them
• when looking up an item, if not found, try again with trailing "_" removed if it has one.
• add to the docs:
  • a note that any attribute can have a trailing "_" to avoid name collisions
  • a quote of the Syntax error for searches to find

If agreed, can we put this in for an upcoming v2.4 release?