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

get_horizons_coord() no longer uses astroquery #7319

Merged
merged 5 commits into from
Dec 7, 2023

Conversation

ayshih
Copy link
Member

@ayshih ayshih commented Nov 23, 2023

Fixes #7080

This PR re-implements get_horizons_coord() so that the function no longer uses astroquery. Consequently, astroquery is no longer a library (optional) dependency, although it is still a dependency for building the gallery examples because we use astroquery to get star positions in one example. This new implementation actually has two improvements:

  • long object names are no longer truncated in the logging output
  • it uses a different HORIZONS API, which is far more permissive of a long list of times (up to 10,000 discretely specified times).

This PR adds requests as an explicit dependency for the core package, but since requests is already an implicit dependency of sunpy.net (via zeep) and of any environment for building docs (via sphinx), it has no impact on most actual environments.

@ayshih ayshih added coordinates Affects the coordinates submodule No Backport A PR that isn't to be backported to any release branch. (To be used as a flag to other maintainers) labels Nov 23, 2023
@ayshih ayshih force-pushed the no_astroquery branch 10 times, most recently from 20f669a to e6229e9 Compare November 27, 2023 21:23
@ayshih ayshih marked this pull request as ready for review November 28, 2023 00:00
@ayshih ayshih requested review from a team as code owners November 28, 2023 00:00
setup.cfg Outdated Show resolved Hide resolved
@ayshih ayshih force-pushed the no_astroquery branch 2 times, most recently from 29de24b to 4d00a4b Compare November 28, 2023 14:14
Copy link
Contributor

@samaloney samaloney left a comment

Choose a reason for hiding this comment

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

Looks good

sunpy/coordinates/ephemeris.py Outdated Show resolved Hide resolved
sunpy/coordinates/ephemeris.py Outdated Show resolved Hide resolved
Copy link
Contributor

@samaloney samaloney left a comment

Choose a reason for hiding this comment

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

LGTM

@nabobalis
Copy link
Contributor

Codecov left a bunch of comments but the patches passed so I am confused but don't care enough to dig down as to what happened.

@nabobalis nabobalis merged commit fe5a0a3 into sunpy:main Dec 7, 2023
23 of 26 checks passed
@ayshih ayshih deleted the no_astroquery branch December 7, 2023 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
coordinates Affects the coordinates submodule No Backport A PR that isn't to be backported to any release branch. (To be used as a flag to other maintainers)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Re-implement get_horizons_coord() to not use astroquery
4 participants