Skip to content

Fallback to remote data files while JSOC is unavailable #346

Merged
nabobalis merged 11 commits intomainfrom
pointing
Jan 8, 2025
Merged

Fallback to remote data files while JSOC is unavailable #346
nabobalis merged 11 commits intomainfrom
pointing

Conversation

@nabobalis
Copy link
Member

@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.

@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
@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.

@nabobalis nabobalis force-pushed the pointing branch 2 times, most recently from f2ef171 to b90144c Compare January 7, 2025 19:28
Co-authored-by: Will Barnes <will.t.barnes@gmail.com>
@nabobalis nabobalis requested a review from wtbarnes January 8, 2025 22:17
@nabobalis nabobalis merged commit db3c654 into main Jan 8, 2025
14 of 16 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