DM-18748: Header items can be present but undefined #17
Conversation
This means that "key in header" is not a sufficient test. Add new support methods to base class to allow translators to test for present and defined in one go.
Otherwise the nice ordered dict mapping to a FITS header will be sorted alphabetically in the YAML file.
|
|
||
| return True | ||
|
|
||
| def is_key_ok(self, keyword): |
There was a problem hiding this comment.
I imagine there is some clever reason that explains why this is necessary/different to is_keyword_defined(), but as it stands, both the docstrings, and the fact that this just directly calls is_keyword_defined() seems odd to me. Is this just a pass-through with a more descriptive method name to allow for neater extension in the future, should it be needed, or something?
There was a problem hiding this comment.
There have to be two different methods because I need the can_translate methods to be able to do the same definedness checks. I'm happy to change the names. is_keyword_defined is a static method that can be used by class methods. is_key_ok is for instances of translations.
There was a problem hiding this comment.
No no, it's fine, that makes sense, thanks!
| @@ -82,7 +82,7 @@ def can_translate(cls, header, filename=None): | |||
| return header["INSTRUME"] == "Hyper Suprime-Cam" | |||
|
|
|||
| for k in ("EXP-ID", "FRAMEID"): | |||
| if k in header: | |||
| if cls.is_keyword_defined(header, k): | |||
There was a problem hiding this comment.
Why do you call is_keyword_defined() here and not is_key_ok() like everywhere else? (I realise this is an odd question having just asserted that there is no difference in functionality)
| @@ -80,7 +80,7 @@ def can_translate(cls, header, filename=None): | |||
| return header["INSTRUME"] == "SuprimeCam" | |||
|
|
|||
| for k in ("EXP-ID", "FRAMEID"): | |||
| if k in header: | |||
| if cls.is_keyword_defined(header, k): | |||
This means that "key in header" is not a sufficient test.