Migrated ESASky module to use EsaTap/PyVO instead of TAPPlus by imbasimba · Pull Request #3572 · astropy/astroquery

imbasimba · 2026-03-31T10:40:07Z

Following the merge of #3511, here is the migration for the ESASky module

codecov · 2026-03-31T10:44:01Z

Codecov Report

❌ Patch coverage is 64.55696% with 28 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.26%. Comparing base (dbf41fd) to head (fca11cc).
⚠️ Report is 117 commits behind head on main.

Files with missing lines	Patch %	Lines
astroquery/esasky/core.py	53.65%	19 Missing ⚠️
astroquery/esasky/__init__.py	76.31%	9 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3572      +/-   ##
==========================================
+ Coverage   73.10%   73.26%   +0.16%     
==========================================
  Files         224      226       +2     
  Lines       20839    21032     +193     
==========================================
+ Hits        15234    15410     +176     
- Misses       5605     5622      +17

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

bsipocz

Big picture looks OK, but we will need to think a bit more about what deprecations will need to be done; or if we want to do a clear break and call this a major enough refactor where we don't provide non-breaking API changes.

bsipocz · 2026-04-03T21:11:41Z

-    urlBase = _config.ConfigItem(
-        'https://sky.esa.int/esasky-tap',
-        'ESASky base URL')
-
-    timeout = _config.ConfigItem(
-        1000,
-        'Time limit for connecting to template_module server.')

-    row_limit = _config.ConfigItem(
-        10000,
-        'Maximum number of rows returned (set to -1 for unlimited).')


These are public attributes, so I don't think we can remove them without doing a deprecation first.

(you can make them an alias to the new variables along with a warning)

bsipocz · 2026-04-03T21:20:27Z

-    def __init__(self, tap_handler=None):
-        super().__init__()
+    def __init__(self, *, show_messages=False, auth_session=None, tap_url=None):
+        super().__init__(auth_session=auth_session, tap_url=tap_url)


OK, so this PR will add some backwards incompatible api changes. If we clearly communicate that in the changelog, I think we can be OK with it even if we decide of not doing a deprecation, but we definitely need to communicate that.

cc @keflavich

bsipocz

I see a lot of remote test failures, changing the review status to make sure those will get fixed up before we merge this.

imbasimba · 2026-04-09T07:41:43Z

Now, I have improved the backward compatibility. I deprecated the old properties and used aliases, restored get_columns, restored and deprecated get_tap, and deprecated parameter tap_handler. Hopefully, that is all the API breaks.
Let me know if I got the property deprecation/aliases right. I’ve never used them myself.

Regarding the failing remote tests, they seem unrelated to the ESASky module and appear to be a cache and hooks issue in astroquery.query. See imbasimba@6266128
Let me know your thoughts on this workaround, and if acceptable, I can merge it into this PR.

imbasimba · 2026-04-27T07:03:45Z

@bsipocz

bsipocz · 2026-04-30T21:44:27Z

@imbasimba - could you separately PR the caching fix?

emellega · 2026-05-04T07:54:25Z

@imbasimba - could you separately PR the caching fix?

Here's a separate PR for the cache fix: #3586

bsipocz

Test failures seem to be related and coming from pyvo, I don't think it has anything to do with the caching bug. Could you please have a look at it? Also, a few comments, but I stopped looking into into the details once noticed the test issues.

bsipocz · 2026-04-30T21:46:33Z

+
+    @urlBase.setter
+    def urlBase(self, value):
+        warnings.warn(


I'm sorry not picking up on this in the first round, but I wonder if we could/should use the deprecated_attribute functionality from astropy.utils instead?

usage is shown e.g. here: astropy/astropy#19593 (comment)

although in these cases the alternative is not another property, so you may want to use the message rather than the alternative argument, and may need to instead follow the example in its documentation

bsipocz · 2026-04-30T21:52:45Z

                 'DWARF_PLANET', 'SPACECRAFT', 'SPACEJUNK', 'EXOPLANET', 'STAR']

-    def __init__(self, *, show_messages=False, auth_session=None, tap_url=None):
+    def __init__(self, *, tap_handler=None, show_messages=False, auth_session=None, tap_url=None):


please use the @deprecated_renamed_argument decorator from astropy.utils instead; plenty of examples for it in the other modules here

Fix some issues with the ESASky tests

emellega · 2026-05-05T11:57:26Z

You are correct, there were some unrelated errors from our backend. We updated our backend recently (in between me writing the code for the "caching fix" and actually posting it as a PR), which broke some of the tests. I have now deployed a fix to our backend. I have also pushed another commit addressing some of the doc tests (mostly just removing validation of the output that changes a lot depending on updates we do in our database almost monthly).

When I run the ESASky remote tests now on this branch I get five errors. All of them show an error that looks something like this:

pyvo.io.vosi.exceptions.W19: ?:?:?: W19: TAP Capabilities must not have an ivo-id other than ivo://ivoa.net/std/TAP

If I apply the fix from the other PR all the ESASky remote tests pass.

I have not yet addressed the other comments you had about how to best deprecate things.

Migrated ESASky module to use pyVO instead of TAPPlus

2a5da5a

Fixed codestyle issues

cb98f6b

bsipocz added the esasky label Apr 3, 2026

bsipocz added this to the 0.4.12 milestone Apr 3, 2026

bsipocz reviewed Apr 3, 2026

View reviewed changes

bsipocz requested changes Apr 4, 2026

View reviewed changes

imbasimba added 4 commits April 6, 2026 13:06

Improved backwards compatibility of ESASky module

30e05fd

Fixed codestyle issues

7e0dee8

Fixed codestyle issues

aeb358a

Added namespaces to aliases

d0d5448

bsipocz reviewed May 5, 2026

View reviewed changes

emellega and others added 2 commits May 5, 2026 13:09

Fix broken tests in ESASky documentation

a0cecbd

Merge pull request #2 from emellega/esasky-pyvo

fca11cc

Fix some issues with the ESASky tests

Uh oh!

Conversation

imbasimba commented Mar 31, 2026

Uh oh!

codecov Bot commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

bsipocz left a comment

Choose a reason for hiding this comment

Uh oh!

bsipocz Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

bsipocz Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

bsipocz Apr 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bsipocz left a comment

Choose a reason for hiding this comment

Uh oh!

imbasimba commented Apr 9, 2026

Uh oh!

imbasimba commented Apr 27, 2026

Uh oh!

bsipocz commented Apr 30, 2026

Uh oh!

emellega commented May 4, 2026

Uh oh!

bsipocz left a comment

Choose a reason for hiding this comment

Uh oh!

bsipocz Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

bsipocz Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

bsipocz Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

emellega commented May 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov Bot commented Mar 31, 2026 •

edited

Loading

bsipocz Apr 3, 2026 •

edited

Loading