Minor issue: errors do not follow intended path due to int conversion fail on empty string

[benkamphaus]

Hi! Thanks for TIA Toolbox, we've found it incredibly useful.

We ran into a minor issue recently I thought I'd report in case it's helpful.

tiatoolbox/tiatoolbox/wsicore/wsireader.py

Lines 135 to 138 in aab2df9

	tuple(int(part) for part in scale.get("version", "").split("."))
	for scale in multiscales
	)
	omero_version = tuple(int(part) for part in omero.get("version", "").split("."))

If the version key is missing, these fail on the attempted type conversion of empty string to integer and throw there rather than reaching an expected error handling path.

[shaneahmed]

Thanks for reporting the issue. Are you able to share sample image or the error or input at this line?

[benkamphaus]

In this case it's a simple error in the Python code:

tuple(int(part) for part in scale.get("version", "").split("."))

The intent of the use of get instead of scale["version"] appears to be for the second parameter default argument, but this argument should default to something that does not immediately throw on split("."), the following code illustrates this, initial dictionary examples:

d1 = {"version": "1.2"}
d2 = {"bad-metadata": "no version"}

Expected outcome when metadata is correct:

[int(v) for v in d1.get("version", "").split(".")]
> [1, 2]

In this case default value for get is unused, so naive key lookup behavior is identical:

[int(v) for v in d1.get["version"].split(".")]
> [1, 2]

When version is missing from metadata, the way this code path is handled raises an error obfuscated by the default param in get:

In [13]: [int(v) for v in d2.get("version", "").split(".")]
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
Cell In[13], line 1
----> 1 [int(v) for v in d2.get("version", "").split(".")]

Cell In[13], line 1, in <listcomp>(.0)
----> 1 [int(v) for v in d2.get("version", "").split(".")]

ValueError: invalid literal for int() with base 10: ''"

or dropping default arg:

----> 1 [int(v) for v in d2.get("version").split(".")]

AttributeError: 'NoneType' object has no attribute 'split'

Without always invalid default arg:

In [14]: [int(v) for v in d2["version"].split(".")]
---------------------------------------------------------------------------
KeyError                                  Traceback (most recent call last)
Cell In[14], line 1
----> 1 [int(v) for v in d2["version"].split(".")]

KeyError: 'version'

This last case is clear about the error (the version key was not there) and does not deterministically encode an uncaught exception in the reading path. Alternatively the case of missing metadata can be explicitly caught and handled.