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

Fallback to remote data files while JSOC is unavailable #346

Merged
merged 11 commits into from
Jan 8, 2025
Merged

Conversation

nabobalis
Copy link
Collaborator

@nabobalis nabobalis commented Dec 11, 2024

The goal here is to remove all of the parsing code and logic from the methods/functions that require a table either from the JSOC or offline files.

The angle here is to make the user pick the table and pass that as input as a new required input.

This avoids us having code for all the various inputs which are possible. Now the user will have to select the table they want via util functions.

TODO:

  • Changelog
  • Migration guide?

@nabobalis nabobalis force-pushed the pointing branch 8 times, most recently from 381a6de to 536ed51 Compare December 11, 2024 20:09
Copy link
Contributor

@wtbarnes wtbarnes left a comment

Choose a reason for hiding this comment

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

I've not left very many useful comments here, but my high-level suggestion would be to refactor this such that get_response_table is unchanged and to instead just pass in the appropriate paths (using the appropriate version number) in the functions that call get_response_table. The same goes for get_error_table.

This is making me realize that all of the logic around how this information is pulled into the higher-level functions is overly complex which is largely my fault. However, this refactor should be independent of what is going on with the JSOC currently.

pyproject.toml Outdated Show resolved Hide resolved
aiapy/util/util.py Outdated Show resolved Hide resolved
aiapy/psf/psf.py Outdated Show resolved Hide resolved
aiapy/data/_manager.py Outdated Show resolved Hide resolved
aiapy/calibrate/util.py Outdated Show resolved Hide resolved
@nabobalis nabobalis force-pushed the pointing branch 2 times, most recently from 0cde18c to fafdbb8 Compare December 17, 2024 01:23
@nabobalis nabobalis changed the title Pointing table Fallback to remote data files while JSOC is unavailable Dec 17, 2024
@LM-SAL LM-SAL deleted a comment from sourcery-ai bot Dec 17, 2024
aiapy/calibrate/meta.py Outdated Show resolved Hide resolved
@nabobalis nabobalis force-pushed the pointing branch 7 times, most recently from 1c30a44 to 1306992 Compare January 1, 2025 04:43
@nabobalis nabobalis force-pushed the pointing branch 5 times, most recently from 602128b to d17c268 Compare January 4, 2025 22:14
@nabobalis nabobalis added Run cron CI Run cron CI on this PR. Run publish Run publish CI on this PR. labels Jan 4, 2025
@nabobalis nabobalis closed this Jan 4, 2025
Copy link
Contributor

@wtbarnes wtbarnes left a comment

Choose a reason for hiding this comment

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

I have reservations about this major of a breaking change, but I don't know of another way. My biggest issue at present is the API for getting different versions of the correction and error tables. I've added comments for a possible alternative.

aiapy/calibrate/tests/test_meta.py Outdated Show resolved Hide resolved
aiapy/calibrate/util.py Outdated Show resolved Hide resolved
aiapy/calibrate/util.py Outdated Show resolved Hide resolved
aiapy/calibrate/util.py Outdated Show resolved Hide resolved
aiapy/calibrate/util.py Outdated Show resolved Hide resolved
changelog/346.breaking.rst Outdated Show resolved Hide resolved
changelog/346.breaking.rst Outdated Show resolved Hide resolved
changelog/346.breaking.rst Outdated Show resolved Hide resolved
changelog/346.breaking.rst Outdated Show resolved Hide resolved
changelog/346.breaking.rst Outdated Show resolved Hide resolved
@nabobalis nabobalis force-pushed the pointing branch 2 times, most recently from f2ef171 to b90144c Compare January 7, 2025 19:28
aiapy/calibrate/util.py Outdated Show resolved Hide resolved
aiapy/calibrate/util.py Outdated Show resolved Hide resolved
aiapy/calibrate/util.py Show resolved Hide resolved
aiapy/calibrate/util.py Outdated Show resolved Hide resolved
aiapy/calibrate/util.py Outdated Show resolved Hide resolved
aiapy/calibrate/util.py Outdated Show resolved Hide resolved
aiapy/calibrate/util.py Outdated Show resolved Hide resolved
aiapy/calibrate/util.py Outdated Show resolved Hide resolved
@nabobalis nabobalis requested a review from wtbarnes January 8, 2025 22:17
@nabobalis nabobalis merged commit db3c654 into main Jan 8, 2025
16 of 18 checks passed
@nabobalis nabobalis deleted the pointing branch January 8, 2025 23:40
@nabobalis nabobalis removed the request for review from wtbarnes January 8, 2025 23:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Run cron CI Run cron CI on this PR. Run publish Run publish CI on this PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants