Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DM-20169: Simplify default search path for fix_header #24

Merged
merged 2 commits into from Jun 17, 2019
Merged

Conversation

timj
Copy link
Member

@timj timj commented Jun 14, 2019

Now a translator can set a class attribute with the location and it will be picked up by the bsae class implementation of translator.search_path().

Add tests for this and stub directories for other translator fixup files.

@timj timj requested a review from SimonKrughoff June 14, 2019 21:59
Now a translator can set a class attribute with the location
and it will be picked up by the bsae class implementation
of translator.search_path().

Add tests for this and stub directories for other translator
fixup files.
If the header is unrecognized by the system we return
without modifying it. This lets generic code request
fixups without requiring that the calling code
knows if the header is understood.
Copy link
Contributor

@SimonKrughoff SimonKrughoff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two small comments.

@@ -207,9 +208,6 @@ def fix_header(header, search_path=None, translator_class=None, filename=None):

Raises
------
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't it still raise a ValueError?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where?

@@ -0,0 +1 @@
DETECTOR: NEW-ID
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought we had agreed (maybe just by convention) that detector ids were integers.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This value is something for me to use in a test of header fixing. It's not being used in metadata translation at all -- I am checking that when a try to fix a header that purports to have this OBSID, that the detector header is replaced with that value.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, in DECam the DETECTOR header refers to the detector serial. It's not the detector name or number.

@timj timj merged commit bde4fab into master Jun 17, 2019
@timj timj deleted the tickets/DM-20169 branch June 17, 2019 22:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants