Refactor astroquery.heasarc to use VO protocols

[zoghbi-a]

This a major refactor of the heasarc module to use the VO interface to the archive. The main motivation is:

• Allow for complex region and table queries.
• Expose the TAP service.
• Cleanup the tests.
• Since the VO interface is the main archive interface, the archive will be able to support this module more.

The main changes inlcude:

• The old class has been renamed HeasarcClass -> HeasarcBrowseClass. The initialized instance is also rename Heasarc -> HeasarcBrowse. The same for the test files.
• The new HeasarcClass class uses an interface similar to those used in other modules e.g. ipac.irsa.
• A deprecation message has been added to the methods used for querying the tables and columns.
• Added the ability to download data from the main heasarc servers, Sciserver and from the cloud.

[pep8speaks]

Hello @zoghbi-a! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

• In the file astroquery/heasarc/core.py:

Line 99:13: W503 line break before binary operator
Line 120:13: W503 line break before binary operator
Line 486:17: W503 line break before binary operator
Line 506:13: W503 line break before binary operator

Comment last updated at 2024-05-15 14:58:22 UTC

[bsipocz]

The old class has been renamed HeasarcClass -> HeasarcBrowseClass. The initialized instance is also renamed Heasarc -> HeasarcBrowse. The same for the test files.

What is the motivation for this? Are there any datasets that are only accessible using the webform, but not the VO backends?

(If the only motivation is to keep what has been here, then it's not necessary, removing everything as part of the refactor is totally fine. Ideally, the old user codes should keep working, but with such a large backend restructure we also have precedence for breaking those)

Doing a proper review may take me until I'm back from the interop.

[zoghbi-a]

All the tables should available through the new API, so it is kept in case some people are using it. If it is ok to remove the old class, I don't see a stopper.

[bsipocz]

so it is kept in case some people are using it

A rename doesn't really solve this scenario, as the continued support would have only worked if the name was kept the same, maybe with a deprecation warning.

So, before diving into a review, I would suggest cleaning up the old class. Maybe try to keep as much of the test examples/docs examples working as possible, or maybe working with a deprecation warning (e.g. in case some of the keywords need to be dropped, or renamed).

[zoghbi-a]

The new class implements most of the useful methods of the old class with a deprecation warning. I will then delete the old class and keep the methods and warnings in the new class.