-
Notifications
You must be signed in to change notification settings - Fork 6
Add ingest_spectra function #168
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
Conversation
This won't pass tests until https://github.com/astrodbtoolkit/astrodb-template-db/pull/160/files is merged. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR moves the ingest_spectrum functionality from SIMPLE into the astrodb_utils package and enhances spectrum ingestion error handling. Key changes include:
- Updates to tests for handling regime matching and ingestion error scenarios.
- New utility function check_obs_date and updates to get_db_regime for improved regime matching.
- Implementation of ingest_spectrum in spectra.py with integration of spectrum validation and database operations.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
tests/test_utils.py | Updated tests for get_db_regime with caplog support for warning checks |
tests/test_spectra.py | Added tests for ingest_spectrum, including error and success cases |
pyproject.toml | Updated dependency version for "ads" |
astrodb_utils/utils.py | Added check_obs_date and improved regime matching logic |
astrodb_utils/spectra.py | Added ingest_spectrum function and integrated spectrum accessibility |
astrodb-template-db | Updated subproject reference |
.github/workflows/run_tests.yml | Modified linter command options |
I don't love this function -- it does a LOT of things. But it works, and we need it. Please review @dr-rodriguez and/or @canavarrete01 . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving in case you need to merge it quickly. We can revisit my suggestions later as they're not strictly needed.
Ingest_spectrum function moved from SIMPLE.