# enable jointcal config writing

XMLWordPrintable

#### Details

• Type: Bug
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
• Story Points:
2
• Sprint:
AP S19-4
• Team:

#### Description

There is a block of code in jointcal that disables the persisting of the config and metadata. We should remove that code and add a dataset description for jointcal configs.

This is the block in question:

 # We don't need to persist config and metadata at this stage. # In this way, we don't need to put a specific entry in the camera mapper policy file def _getConfigName(self): return None   def _getMetadataName(self): return None 

#### Activity

Hide
Eli Rykoff added a comment -

I think that we want to resurrect this issue now that we're looking to put jointcal into production mode. The config saving shouldn't be too difficult, but I don't know if needs a tract/filter template.

Show
Eli Rykoff added a comment - I think that we want to resurrect this issue now that we're looking to put jointcal into production mode. The config saving shouldn't be too difficult, but I don't know if needs a tract/filter template.
Hide
John Parejko added a comment - - edited

Eli Rykoff: Ask, and ye shall receive (and be marked reviewer)! I've only implemented the config persistence here, as I don't know if we need metadata persistence yet, and the metadata part would need a more complicated template. I think the main thing we'd want the template for is tracking runtime metrics, which would be good, but we're not actually trying to do it yet.

Show
John Parejko added a comment - - edited Eli Rykoff : Ask, and ye shall receive (and be marked reviewer)! I've only implemented the config persistence here, as I don't know if we need metadata persistence yet, and the metadata part would need a more complicated template. I think the main thing we'd want the template for is tracking runtime metrics, which would be good, but we're not actually trying to do it yet. Jenkins run: https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/29517/pipeline
Hide
Eli Rykoff added a comment -

Of course the obs_base PR didn't show up on Jira. Sigh.

I'm glad I thought to look for it because I believe it has the wrong python class. And if the tests pass in spite of that, then there might need to be some additional check in the tests that the persisted datatype is correct, perhaps by reading it in and not just checking its existence. Eg:
jointcalConfig = butler.get('jointcal_config') and checking the class of the returned value.

Show
Eli Rykoff added a comment - Of course the obs_base PR didn't show up on Jira. Sigh. I'm glad I thought to look for it because I believe it has the wrong python class. And if the tests pass in spite of that, then there might need to be some additional check in the tests that the persisted datatype is correct, perhaps by reading it in and not just checking its existence. Eg: jointcalConfig = butler.get('jointcal_config') and checking the class of the returned value.
Hide
John Parejko added a comment -

Jira's picking up the obs_base PR now.

Good catch on that. I just copied and pasted another Config and didn't even think to look at that line. Turns out, the Butler will raise if you try to read it, because the config file itself has an assert on the type of the config. I added some other tests to check that it's actually written with the test values. Please take another look.

Show
John Parejko added a comment - Jira's picking up the obs_base PR now. Good catch on that. I just copied and pasted another Config and didn't even think to look at that line. Turns out, the Butler will raise if you try to read it, because the config file itself has an assert on the type of the config. I added some other tests to check that it's actually written with the test values. Please take another look.
Hide
John Parejko added a comment -

Thanks for the review. If we need metadata persistence, we can file a new ticket.

Merged and done.

Show
John Parejko added a comment - Thanks for the review. If we need metadata persistence, we can file a new ticket.  Merged and done.

#### People

Assignee:
John Parejko
Reporter:
John Parejko
Reviewers:
Eli Rykoff
Watchers:
Colin Slater, Eli Rykoff, John Parejko, John Swinbank, Simon Krughoff