Add Auto header detection to Populate_Metadata.py - #232
Conversation
sbesson
left a comment
There was a problem hiding this comment.
My main concern with the proposed implementation is the brittle nature of using nested imports to try and bisect the version of the omero-metadata package.
With the modern Python requirements (Python 3.10+) of the OMERO environment, we should be able to use standard libraries such as importlib.metadata for introspecting the presence and the version of the omero-metadata package and packaging for checking the minimal version
>>> from importlib.metadata import version
>>> from packaging.version import Version
>>> version('omero_metadata')
'0.13.0'
>>> Version(version('omero_metadata')) >= Version('0.11')
True
>>> Version(version('omero_metadata')) >= Version('0.14')
False
>>> A second question is: what should happen if the minimal requirements are not met. This PR maintains the current statu quo of supporting multiple paths (no omero-metadata, omero-metadata<0.11 and omero-metadata>=0.11).
An alternative thought especially in the context of the upcoming OMERO.server 5.7.0 milestone would be to put a hard requirement of this script on omero-metadata 0.11+ being installed server-side and make sure this is documented appropriately
| try: | ||
| # Hopefully this will import | ||
| # https://github.com/ome/omero-metadata/blob/v0.3.1/src/populate_metadata.py | ||
| # Automatic header detection requires omero-metadata >= 0.11.0. |
There was a problem hiding this comment.
Given the modern Python requirements (Python 3.10+), I wonder if we want to start using importlib.metadata for introspecting the package version and packaging for checking a minimal version
>>> from importlib.metadata import version
>>> from packaging.version import Version
>>> version('omero_metadata')
'0.13.0'
>>> Version(version('omero_metadata')) >= Version('0.11')
True
>>> Version(version('omero_metadata')) >= Version('0.14')
False
>>>
I noticed that much of the work ome/omero-metadata#67 to create an auto header detecting method and allow NaNs was not propagated to the
Populate_Metadata.pyscript.This PR proposes adding this functionality such that the header type is automatically detected and "allow_nan" is an option.
To test, use a working CSV without header type defined such as:
Without PR, all the columns should be
StringColumn. With PR:if a
#headerrow is defined in the CSV, it should always take precedence over the auto-detection.