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-46312: Migrate unit tests to ComCamSim data #199

Merged
merged 5 commits into from
Sep 16, 2024
Merged

Conversation

kfindeisen
Copy link
Member

This PR migrates the unit test repo from DECam to ComCamSim, and cleans up some related technical debt.

@kfindeisen kfindeisen marked this pull request as ready for review September 16, 2024 20:18
@kfindeisen kfindeisen changed the title DM-46312: Prompt Processing preload on w_2024_37 works for DECam and LATISS but not HSC or ComCamSim DM-46312: Migrate unit tests to ComCamSim data Sep 16, 2024
Copy link
Collaborator

@hsinfang hsinfang left a comment

Choose a reason for hiding this comment

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

LGTM -- thanks for updating the test repo.

tests/data/make_central_repo.sh seems missed in this update. I've found that file valuable in re-constructing or modifying the test repo, which we do need to do from time to time.

@kfindeisen
Copy link
Member Author

Oops, so that's where export.yaml was used! I'll fix it.

@hsinfang
Copy link
Collaborator

Another comment: with this update, we no longer need setupRequired(obs_decam) in ups/prompt_processing.table, I think.

DECam is increasingly unrepresentative of expected LSST data, and the
original dataset (adapted from ap_verify) had a nonstandard repository
layout that had to be special-cased. ComCamSim should provide a more
realistic data set going forward.

The unit test repository remains at dimensions-config 6.
This commit also removes all dimension records relating to the DECam
instrument or the decam_rings_v1 skymap from the registry.
Tests previously used a mock pipeline because no preprocessing tasks
had been implemented, and empty pipelines are invalid.
@kfindeisen
Copy link
Member Author

@hsinfang would you like to take a look at make_central_repo.sh? I ended up almost completely rewriting it.

@hsinfang
Copy link
Collaborator

Looks great to me, and it will also serve as a good reference if later we want to build a new repo with another dataset. Thank you!

@kfindeisen kfindeisen merged commit 4904cbd into main Sep 16, 2024
5 checks passed
@kfindeisen kfindeisen deleted the tickets/DM-46312 branch September 16, 2024 23: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.

2 participants