ESA HST module PyVO migration #3367

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.27%. Comparing base (1becbba) to head (3f6d625).
⚠️ Report is 33 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.27%   +0.19%     
==========================================
  Files         232      232              
  Lines       19890    19897       +7     
==========================================
+ Hits        13940    13983      +43     
+ Misses       5950     5914      -36

☔ 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

astroquery/esa/hubble/core.py

        if show_messages:
            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

astroquery/esa/hubble/core.py

+    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

astroquery/esa/hubble/core.py

+            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

astroquery/esa/hubble/core.py

    @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

astroquery/esa/hubble/core.py

@@ -1061,4 +1086,5 @@ def get_datalabs_path(self, filename, default_volume=None):
        return full_path


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

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

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

	# 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!

ESA HST module PyVO migration #3367

Are you sure you want to change the base?

ESA HST module PyVO migration #3367

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!

Uh oh!

codecov bot commented Jul 3, 2025 •

edited

Loading

JorgeHigueraFombuena commented Aug 4, 2025 •

edited

Loading