# Cleanup ci_hsc_gen2 to use new convert script instead of custom one

XMLWordPrintable

## Details

• Type: Story
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
None
• Story Points:
2
• Sprint:
AP S20-3 (February), AP S20-4 (March)
• Team:
• Urgent?:
No

## Description

In order to help get DM-22655 merged sooner, I'm going to move the work of converting ci_hsc to use the new script onto this ticket, instead of doing it there. I'd already started on that process, and will move that work onto this ticket.

## Activity

Hide
Tim Jenness added a comment -

John Parejko has asked me to take the ticket out of reviewed state because I don't think the ticket should be merged when it is so much slower than the previous implementation.

Looking at the code the only difference I can see between the new and the old version is that the obs_base conversion script does not set rootSkyMapName in the config. Maybe that causes the problem?

Show
Tim Jenness added a comment - John Parejko has asked me to take the ticket out of reviewed state because I don't think the ticket should be merged when it is so much slower than the previous implementation. Looking at the code the only difference I can see between the new and the old version is that the obs_base conversion script does not set rootSkyMapName in the config. Maybe that causes the problem?
Hide
John Parejko added a comment -

Thanks for that suggestion, Tim Jenness: I don't understand why rootSkyMapName mattered (the skymap config and name was already specified in config.skymaps), but it appears to have cut the convert runtime down to about a minute.

Show
John Parejko added a comment - Thanks for that suggestion, Tim Jenness : I don't understand why rootSkyMapName mattered (the skymap config and name was already specified in config.skymaps), but it appears to have cut the convert runtime down to about a minute.
Hide
Tim Jenness added a comment - - edited

That's great. I haven't looked too deeply at the code but I saw a bunch of "if rootSkyMapName is None" shenanigans going on suggesting that different code paths would be involved.

Show
Tim Jenness added a comment - - edited That's great. I haven't looked too deeply at the code but I saw a bunch of "if rootSkyMapName is None" shenanigans going on suggesting that different code paths would be involved.
Hide
Jim Bosch added a comment -

I saw a bunch of "if rootSkyMapName is None" shenanigans going on suggesting that different code paths would be involved.

This is a workaround for the fact that a Gen2 repo can have datasets defined on tract and patch without having a skymap dataset that defines what those mean.  It's a totally ill-formed repository configuration, and apparently we get away with it because people just guess that a skymap dataset in a child repository is the right one for the parent.  That config option sets what skymap to guess when the information simply isn't in the Gen2 repo.

Show
Jim Bosch added a comment - I saw a bunch of "if rootSkyMapName is None" shenanigans going on suggesting that different code paths would be involved. This is a workaround for the fact that a Gen2 repo can have datasets defined on tract and patch without having a skymap dataset that defines what those mean.  It's a totally ill-formed repository configuration, and apparently we get away with it because people just guess that a skymap dataset in a child repository is the right one for the parent.  That config option sets what skymap to guess when the information simply isn't in the Gen2 repo.
Hide
John Parejko added a comment -

Jenkins failure due to the bug introduced by DM-24285, but I think everything in this ticket is ok. I've merged it to get this done with. If there is a bug hiding because of the problem in DM-24285, let me know and I'll fix it ASAP.

Show
John Parejko added a comment - Jenkins failure due to the bug introduced by DM-24285 , but I think everything in this ticket is ok. I've merged it to get this done with. If there is a bug hiding because of the problem in DM-24285 , let me know and I'll fix it ASAP.

## People

• Assignee:
John Parejko
Reporter:
John Parejko
Reviewers:
Tim Jenness
Watchers:
Jim Bosch, John Parejko, John Swinbank, Tim Jenness