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

Refactored query criteria to use ehst.archive table #2524

Merged

Conversation

javier-ballester
Copy link
Contributor

Dear Astroquery team,

The query_criteria method in Hubble astroquery is using a complex query, joining three big tables and this makes its execution really slow. This query has been refactored to use only ehst.archive table, which contains all the criteria parameters.

Please let me know if any further changes are necessary.

Kind regards,

Javier

@javier-ballester javier-ballester marked this pull request as ready for review September 12, 2022 12:54
@codecov
Copy link

codecov bot commented Sep 12, 2022

Codecov Report

Merging #2524 (42eea5f) into main (a750c43) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #2524   +/-   ##
=======================================
  Coverage   63.08%   63.08%           
=======================================
  Files         133      133           
  Lines       17313    17313           
=======================================
  Hits        10922    10922           
  Misses       6391     6391           
Impacted Files Coverage Δ
astroquery/esa/hubble/core.py 86.47% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@bsipocz bsipocz added this to the v0.4.7 milestone Sep 12, 2022
Copy link
Member

@bsipocz bsipocz left a comment

Choose a reason for hiding this comment

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

One minor thing with the changelog that I'm happy to fix myself prior merging.

However, I now see a relevant doctest failure in docs/esa/hubble/hubble.rst (it maybe just the one it first stopped), please go through those as part of this PR.

python setup.py test -t docs/esa/hubble --remote-data

@bsipocz
Copy link
Member

bsipocz commented Sep 13, 2022

Thanks @javier-ballester!

@bsipocz bsipocz merged commit 01d75d6 into astropy:main Sep 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants