ESA HST module PyVO migration by JorgeHigueraFombuena · Pull Request #3367 · astropy/astroquery

JorgeHigueraFombuena · 2025-07-03T09:22:37Z

Dear astroquery team, this ticket aims to update the HST to make use of the PyVO module.

Happy to contribute!!

…uest method

codecov · 2025-07-03T09:25:55Z

Codecov Report

❌ Patch coverage is 90.56604% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.48%. Comparing base (1becbba) to head (b918da4).
⚠️ Report is 273 commits behind head on main.

Files with missing lines	Patch %	Lines
astroquery/esa/hubble/core.py	90.69%	4 Missing ⚠️
astroquery/esa/utils/utils.py	75.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3367      +/-   ##
==========================================
+ Coverage   70.08%   70.48%   +0.39%     
==========================================
  Files         232      232              
  Lines       19890    19977      +87     
==========================================
+ Hits        13940    14080     +140     
+ Misses       5950     5897      -53

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

jespinosaar · 2025-07-03T12:24:41Z

Hi @JorgeHigueraFombuena , great to see your first PR to Astroquery! @bsipocz , he is a new team member at ESA and I have been supporting him on this branch.

It seems all the checks are passing now, great! Please let us know if you have comments or concerns. Thanks!

bsipocz · 2025-07-03T14:39:16Z

Welcome @JorgeHigueraFombuena! And straight into the deep end.

I have some deadlines to meet before the long weekend and will be at a conference next week. But I'll try to get this reviewed quickly (passing the tests is already a great sign, but I expect to have some comments on making the API more consistent with the other modules).

JorgeHigueraFombuena · 2025-07-03T15:10:50Z

Hi @bsipocz

I hope the code is clear enough to make reviewing easy!
Thanks!

JorgeHigueraFombuena · 2025-07-21T07:08:53Z

Hi @bsipocz !
Did you have time to review the pull request?

bsipocz · 2025-07-21T18:09:55Z

@JorgeHigueraFombuena - There is another big PR in front of this one, hopefully I can clear both this week.

JorgeHigueraFombuena · 2025-07-22T07:11:58Z

@bsipocz
Thank you very much!

bsipocz

The property/attribute renames are blockers for the merge, the rest of the inline comments are minor.

Also, I do see 2 test failures (on main, too, so it's separate), but two other ones triggered only with this PR. Could you please have a look into them?
(pytest -P esa.hubble -R should pick them up once all the test requirements are installed)

=============================================== short test summary info ================================================
FAILED astroquery/esa/hubble/tests/test_esa_hubble_remote.py::TestEsaHubbleRemoteData::test_query_tap_async - pyvo.dal.exceptions.DALServiceError: 500 Server Error: 500 for url: https://hst.esac.esa.int/tap-server/tap/async/0...
FAILED astroquery/esa/hubble/tests/test_esa_hubble_remote.py::TestEsaHubbleRemoteData::test_cone_search - AttributeError: 'ESAHubbleClass' object has no attribute '_tap'
FAILED astroquery/esa/hubble/tests/test_esa_hubble_remote.py::TestEsaHubbleRemoteData::test_get_datalabs_path_image - UserWarning: File ib4x04ivq_flt.jpg is not accessible. Please ensure the WFC3/UVIS volume is mounted in your ESA Da...
FAILED astroquery/esa/hubble/tests/test_esa_hubble_remote.py::TestEsaHubbleRemoteData::test_get_datalabs_path_fits - UserWarning: File ib4x04ivq_flt.fits.gz is not accessible. Please ensure the WFC3/UVIS volume is mounted in your ES...

bsipocz · 2025-07-31T16:48:33Z

            self.get_status_messages()

+    @property
+    def vo(self) -> pyvo.dal.TAPService:


call this tap to be consistent with the other modules

bsipocz · 2025-07-31T16:49:32Z

+    def __init__(self, *, show_messages=True, auth_session=None):
+        super().__init__()
+
+        self._vo = None


call this self._tap to be consistent with the other modules

bsipocz · 2025-07-31T16:52:30Z

+            while job.phase == 'EXECUTING':
+                time.sleep(3)


I don't think you need to add a manual sleep in here. Also, why not just run it with run_async instead of doing the submission/run/fetch cycle? (genuine question, you may have good reasons to prefer this way)

I've changed it to the method you proposed, and it works fine. Thank you!

bsipocz · 2025-07-31T16:55:39Z

    @deprecated(since="0.4.7", alternative="query_tap")
    def query_hst_tap(self, query, *, async_job=False, output_file=None,


unrelated so can be done in a separate PR, but given this backend change, the deprecations could be also cleaned up (but we can also keep them in longer if you prefer that)

The method was cleaned up!

bsipocz · 2025-07-31T16:56:47Z



-ESAHubble = ESAHubbleClass()
+# Neet to be false in order to avoid errors in tests


I would rephrase

Suggested change

# Neet to be false in order to avoid errors in tests

# Need to be False in order to avoid reaching out to the remote server at import time

JorgeHigueraFombuena · 2025-08-04T15:25:49Z

Hi @bsipocz !
We are currently working on the issue with the remote tests and need more time to resolve it.
In the meantime, the rest of the comments have been addressed.
Regarding the autochecks, I see that one of them is still failing, but I'm not sure how to fix it.

JorgeHigueraFombuena · 2025-08-18T07:06:50Z

Hi @bsipocz
We've already finished the changes in our side.
All the remote tests are working fine!
So this PR could be reviewed again
Thanks and sorry for the delay

bsipocz

Thank you @JorgeHigueraFombuena! I have two very minor comments, will just add them and then merge this PR.

Jorge Higuera added 5 commits June 12, 2025 15:39

EHSTPCR-1138 Migrating to pyvo

6310540

EHSTPCR-1138 Refactor tests to integrate pyvo library

6d5c16b

EHSTPCR-1138 Removing TAPCLIENT parameter

10cbc34

EHSTPCR-1138 Removing unnecessary code

8909370

EHSTPCR-1138 Adding new parserMethod parameter to execute_servlet_req…

f42db67

…uest method

Jorge Higuera added 4 commits July 3, 2025 11:40

EHSTPCR-1138 Fixing code style issues and changelog issue

d02b197

EHSTPCR-1138 Fixing code style issues and tests coverage

d7f74fa

EHSTPCR-1138 Adding code coverage

fd0b074

EHSTPCR-1138 Fixing code style

769be07

bsipocz added the esa.esa_hubble label Jul 3, 2025

bsipocz self-requested a review July 3, 2025 17:54

bsipocz added this to the v0.4.11 milestone Jul 31, 2025

bsipocz requested changes Jul 31, 2025

View reviewed changes

Jorge Higuera added 4 commits August 4, 2025 16:18

EHSTPCR-1138 Fixing comments of PR

1f15900

EHSTPCR-1138 Removing deprecated code

49406a6

EHSTPCR-1138 Fixing code style

f029c47

EHSTPCR-1138 Fixing code style

3f6d625

JorgeHigueraFombuena requested a review from bsipocz August 28, 2025 13:46

bsipocz approved these changes Sep 4, 2025

View reviewed changes

Comment thread astroquery/esa/hubble/core.py Outdated

Comment thread CHANGES.rst Outdated

bsipocz reviewed Sep 4, 2025

View reviewed changes

Comment thread astroquery/esa/hubble/core.py Outdated

Very minor comments from final review

abbb5ef

Cleanup unused DummyTapHandler

b918da4

bsipocz force-pushed the ESA_ehst-pyvo_migration branch from f80e854 to b918da4 Compare September 4, 2025 03:47

bsipocz merged commit 4904296 into astropy:main Sep 4, 2025
12 checks passed

bsipocz mentioned this pull request Sep 16, 2025

BUG: doctest execution is stuck on hubble.rst #3412

Closed

		@deprecated(since="0.4.7", alternative="query_tap")
		def query_hst_tap(self, query, *, async_job=False, output_file=None,



		ESAHubble = ESAHubbleClass()
		# Neet to be false in order to avoid errors in tests

	# Neet to be false in order to avoid errors in tests
	# Need to be False in order to avoid reaching out to the remote server at import time

Uh oh!

Uh oh!

Conversation

JorgeHigueraFombuena commented Jul 3, 2025

Uh oh!

codecov Bot commented Jul 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

jespinosaar commented Jul 3, 2025

Uh oh!

bsipocz commented Jul 3, 2025

Uh oh!

JorgeHigueraFombuena commented Jul 3, 2025

Uh oh!

JorgeHigueraFombuena commented Jul 21, 2025

Uh oh!

bsipocz commented Jul 21, 2025

Uh oh!

JorgeHigueraFombuena commented Jul 22, 2025

Uh oh!

bsipocz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JorgeHigueraFombuena commented Aug 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JorgeHigueraFombuena commented Aug 18, 2025

Uh oh!

bsipocz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov Bot commented Jul 3, 2025 •

edited

Loading

JorgeHigueraFombuena commented Aug 4, 2025 •

edited

Loading