Skip to content

Add support for Astra model files#96

Merged
albireox merged 10 commits into
mainfrom
albireox-astra-model-spectra
Jun 30, 2026
Merged

Add support for Astra model files#96
albireox merged 10 commits into
mainfrom
albireox-astra-model-spectra

Conversation

@albireox

@albireox albireox commented Jun 25, 2026

Copy link
Copy Markdown
Member
  • Adds support for astraVisit[pipeline] and astraStar[pipeline] model files.
  • Fixes an issue with byte swapping in Numpy 2.
  • Adds a flag to AstraProducts to indicate files that exist but don't have data.

Currently the uv.lock file is out of date because dependency resolution fails due to an incompatibility between the solara versions in sdss-solara and sdss-explorer.

@albireox albireox marked this pull request as ready for review June 25, 2026 22:19
@albireox albireox requested a review from havok2063 as a code owner June 25, 2026 22:19

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds support for additional Astra model spectra products and augments Astra pipeline metadata so clients can tell whether a product file exists and whether it contains any data, alongside dependency/runtime bumps intended to support newer library versions (notably NumPy/Astropy).

Changes:

  • Extend the model spectra datamodel JSON to include Astra astraStar* and astraVisit* products.
  • Update Astra pipeline metadata (get_pipe_meta) to include a has_data flag per product and to only include files with data in the returned file list.
  • Bump runtime/dependency requirements (Python >= 3.11, Astropy >= 7.2) and adjust byte-order handling in spectral extraction for NumPy 2.

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
python/valis/io/spectra.py Updates endianness handling when extracting spectral arrays (NumPy 2 compatibility).
python/valis/io/model_spectra_info.json Adds datamodel entries for new Astra model file products.
python/valis/db/queries.py Expands Astra products returned by get_pipe_meta and adds has_data detection via FITS inspection; adjusts path-existence behavior for BOSS/APOGEE.
python/valis/db/models.py Adds has_data to the Astra product response model.
pyproject.toml Raises minimum Python version and bumps Astropy dependency floor.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread python/valis/io/spectra.py
Comment thread python/valis/db/queries.py
Comment thread python/valis/db/queries.py
Comment thread python/valis/db/queries.py
Comment thread python/valis/db/queries.py
Comment thread python/valis/db/queries.py
@albireox

albireox commented Jun 30, 2026

Copy link
Copy Markdown
Member Author

I think this is ready for review and merging.

The tests are failing because of this issue in pylama. Not sure if pylama is used in valis, and it seems it has not seen any releases in 4 years.

@havok2063

Copy link
Copy Markdown
Contributor

The tests are failing because of this issue in pylama. Not sure if pylama is used in valis, and it seems it has not seen any releases in 4 years.

Not directly. It's a dev dependency from linting/testing. You can try removing it from the pyproject.toml file and see if things pass. Some of those dev deps are from back in the pylint, pyflakes days and probably could be cleaned up with ruff.

@albireox

Copy link
Copy Markdown
Member Author

I removed pylama. Ruff produces 103 errors, most of them line lengths and import order which are probably easy to fix, but that seems like a different PR.

I had to change test_targets::test_resolve_target_coord to expect 3 results, but since that queries Simbad I assume things just changed upstream.

@havok2063

Copy link
Copy Markdown
Contributor

I removed pylama. Ruff produces 103 errors, most of them line lengths and import order which are probably easy to fix, but that seems like a different PR.

Yeah that can be a different PR.

I had to change test_targets::test_resolve_target_coord to expect 3 results, but since that queries Simbad I assume things just changed upstream.

Interesting. Likely so. As long as the test still passes.

@albireox albireox merged commit ee5ac95 into main Jun 30, 2026
2 checks passed
@albireox albireox deleted the albireox-astra-model-spectra branch June 30, 2026 16:00
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.

3 participants