While the Gen 2 stub repository doesn't have transmission curve datasets, it should be possible to manually add them (using https://github.com/lsst/obs_subaru/blob/master/bin.src/installTransmissionCurves.py) before conversion to Gen 3.

EDIT: the Gen 2->3 conversion script adds the transmission curves automatically

I'm expanding the scope of this issue to also cover ap_verify_hits2015 and its CI version. I need to test the Gen 2-3 conversion script (DM-21915) on both HSC and DECam anyway, and once the script is working adding Gen 3 support for all the repos is no more work than adding it for just one.

Hi Jim Bosch, this review involves five packages:

• obs_base, bugfix to the converter
• ap_verify_dataset_template, the dataset conversion script
• ap_verify_ci_hits2015, ap_verify_hits2015, ap_verify_ci_cosmos_pdr2, the datasets being given Gen 3 repositories and matching config files

Naturally, I'm only asking you to review the code in the first two packages (especially the dataset conversion script's use of the existing repository converter), though the datasets may be useful as example output.

For those keeping track, I have not converted ap_verify_testdata because it depends on obs_test, which is incompatible with Gen 3. It may be possible to convert it after DM-24844.

I'm sorry but I don't understand how obs_test can be involved in anything that involves data from a real instrument? If we have lots of data for a pretend instrument in obs_test then we should fix it but I was under the impression that obs_test was a hacked together pretend instrument.

Tim Jenness, obs_test has a couple of images from old lsstSim, per the readme. I am not aware of any requirement that data be from a real instrument.

Yes, I know that obs_test has a couple of old sims images. What I'm confused by is that ap_verify_testdata depends on obs_test.

All looks good; I didn't have much to say, but there are a few comments on the _templates PR.

I did not look at the collection structure in the converted repos - I think the code you're using to create them probably guarantees that they're close enough to what we're envisioning that it's not worth holding anything up right now. As a heads up I think we'll want to have another pass at defining those conventions more clearly in the future (RFC-663 is clearly proving to be at least insufficient, and in some cases maybe made poor choices), so these probably shouldn't be considered set in stone (and hopefully the code you delegate to will continue to make changes there propagate through to here automatically - though that may not be the case for code that uses the datasets created here.

It's a bit unfortunate that daf_butler and ap_verify use "dataset" to mean very specific and quite different things, but I'm sure this is now too baked in to both to change, and it's not completely surprising given that the meaning of the word outside our codebase is somewhat ambiguous.

Re. ap_verify_testdata depending on obs_test, I believe we did that because we needed some manner of "real" image data to run tests on and wanted to keep the testdata package from being any larger than it needed to be. Since perfectly acceptable test images already exist, and we didn't want to tie ap_verify_testdata to any particular obs_ package, we went with "generic" image data in obs_test. This could be pretty easily circumvented by putting a copy of the data that's currently loaded from obs_test directly in the ap_verify_testdata repo.

Thanks for the review! For the record, the current reference for ap_verify datasets' collection structure is the discussion in DM-21915 (I just realized that I should also document that in ap_verify_dataset_templates, since its documentation is the "official" spec for what an ap_verify dataset should look like).

As for the term "dataset", yes, it's been a problem since the Gen 2 days. I could swear we had a Slack discussion about it a few years back, but now I can't find it.