Ability to not throw exceptions for un-declared Dataset attributes?

[richard-moss]

Problem

When dealing with Datasets, I'm (like many I would guess) always faced with the issue of wanting to get a tag value, but of course having to deal with handling the real-world dicom case that the tag isn't present in the Dataset.

For example, some simple code to print the Accession number out of a Dataset (call it dcm):

if hasattr(dcm, 'AccessionNumber'):
   print(dcm.AccessionNumber)

or maybe

acc_num = getattr(dcm, 'AccessionNumber', '')
print(acc_num)

...you could also put the print in a try-except and probably many other ways of handling this

Question

Is there a more elegant way of handling this? (meaning less verbage, more concise, cleaner,...)

I could of course subclass Dataset and override __getattr__ to return None if the attribute doesn't exists, but is there any 'default' way of handling the above lookup better?

(Sub-classing is elegant in one sense, but a problem then if you forget to cast a Dataset to your sub-class, etc)

Observation

The 'reverse' is true for Datasets (or any class object) - you can assign to a (previously) un declared attribute without any problems:

dcm.AccessionNumber = 'AC1234'

Idea

Would it be reasonable to have a config or ability in Dataset's to have the default lookup of a un-declared attribute return None if not declared?

[darcymason]

Is there a more elegant way of handling this? (meaning less verbage, more concise, cleaner,...)

There is in, shown just below, or get further below:

if 'AccessionNumber' in dcm:
    ....

Would it be reasonable to have a config or ability in Dataset's to have the default lookup of a un-declared attribute return None if not declared?

That's the purpose of the get method, e.g.:

dcm.get("AccessionNumber")

get returns None by default, but you could supply an alternate value

[mrbean-bremen]

There is a couple of caveats with using get: it would also return None for any illegal tag name, and you can't differentiate between a missing and an empty tag (though that hardly matters in reality). Apart from that, get is probably what you want.

[darcymason]

There is a couple of caveats with using get: it would also return None for any illegal tag name, and you can't differentiate between a missing and an empty tag (though that hardly matters in reality). Apart from that, get is probably what you want.

That's a good point - and thus the in operator (or hasattr) would be the most explicit. Alternatively one could use a non-None default value for get and check for that.

For printing summaries of files, I've often used this kind of pattern:

acc_num = dcm.get("AccessionNumber", "N/A")

for any values that might not be guaranteed to be there (and even some that might be required, if the file is not conformant).

Checking for non-existent or null values is always a pain...

[richard-moss]

This is also one of primary use cases, logging attributes. ...now knowing about get() makes this all much simpler/cleaner - thx

[darcymason]

An extension of that pattern (without intermediate variable names) that works well is:

required_keywords =  ['PatientName', 'PatientID', 'AccessionNumber', "Rows", "Columns"]    # etc, ...
for kw in required_keywords:
    print(f'{kw}: {ds.get(kw, "N/A")}')

Or could collect them in a dict for whatever kind of output, e.g.

my_results = {kw: ds.get(kw, "N/A") for kw in required_keywords}

[richard-moss]

yeah - that's a nice pattern!
...Throw it into a dict based on tag name, with required format to use to print in the values and you've got a really concise way of doing specifically formatted tag output (great for things like overlays)

[darcymason]

Would it be reasonable to have a config or ability in Dataset's to have the default lookup of a un-declared attribute return None if not declared?

It is tempting, but this would not be standard Python behavior (explicit is better than implicit). However, I suppose if it threw an error for non-DICOM-keywords to catch typos, then it wouldn't be much different than how the in operator now works. Perhaps I could be swayed if someone could show code snippets where it really helped - in any of these cases, there are always branches to handle the missing values, so I'm not clear how much it would help the overall code clarity.

[darcymason]

@richard-moss, note I have moved the issue to a Discussion ... it looks like email notifications show the issue as closed without noting that, so pinging you just in case...

[richard-moss]

Great thanks Darcy - I wasn’t sure where to post it

…

On Apr 16, 2021, at 12:50 PM, Darcy Mason ***@***.***> wrote:  @richard-moss, note I have moved the issue to a Discussion ... it looks like email notifications show the issue as closed without noting that, so pinging you just in case... — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[richard-moss]

To be honest I hadn't realized there was get()! mea cupla

@darcymason You're right - this is probably what I want to be using, as it fits the use case. You're right, I could use xxx in Dataset, but that's just an alternate way of doing a hasattr type check, which I was trying to avoid using to avoid code-bloat having to do it everywhere.

Actually the behaviour of returning None even for non-dicom elements is actually good for me as well.