LSSTDESC / skyCatalogs

Create sky catalogs and provide access via API
BSD 3-Clause "New" or "Revised" License
6 stars 5 forks source link

Add support for Roman prism and grism bands #104

Open arunkannawadi opened 5 months ago

arunkannawadi commented 5 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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.