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

Add support for Roman prism and grism bands #104

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

arunkannawadi
Copy link
Member

Addresses #103 .

This is currently a draft PR since I think some upstream changes in GalSim may be required as well. Are there any tests that I should include in the mean time?

@arunkannawadi
Copy link
Member Author

Now that GalSim v2.6 is released, this PR is ready to be reviewed and merged. Should we require galsim>=2.6 somewhere? I don't see it listed in pyproject.toml.

@JoanneBogart
Copy link
Collaborator

JoanneBogart commented Sep 21, 2024

galsim (and many other packages) are not listed in pyproject.toml because, for the most part, skyCatalogs is used in the context of LSST Science Pipelines, which includes galsim. Recent builds of LSST Science Pipelins are at galsim version 2.5.3. I don't want to put in a requirement which is necessary only for Roman and troublesome for Rubin unless there is a way to signal that it should apply only when requested. If that's not possible your code could check the galsim version and behave accordingly.

@arunkannawadi
Copy link
Member Author

Ah, that's a fair point. rubin-env with updated version of galsim is not going to appear for a long time (likely not for another year), so I can add appropriate checks that can work with both old and new versions.

Btw, is there any expectation that skyCatalogs should work with earlier versions of rubin-env?

@JoanneBogart
Copy link
Collaborator

JoanneBogart commented Sep 24, 2024

Generally speaking yes, it should be compatible with older versions of galsim within reason, but it would be ok with me to set some limit; I just wouldn't want to go to 2.6 quite yet.
When new parquet files are written, they have embedded metadata which includes galsim version, so at least you can see after the fact what was used.

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