Enable TAP for ESO module #3141
Conversation
|
Hello @juanmcloaiza! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2025-01-17 14:13:03 UTC |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3141 +/- ##
=======================================
Coverage ? 67.96%
=======================================
Files ? 230
Lines ? 18405
Branches ? 0
=======================================
Hits ? 12509
Misses ? 5896
Partials ? 0 ☔ View full report in Codecov by Sentry. |
astroquery/eso/core.py
Outdated
| try: | ||
| table_to_return = tap.search(query_str).to_table() | ||
| except pyvo.dal.exceptions.DALQueryError: | ||
| raise pyvo.dal.exceptions.DALQueryError(f"\n\nError executing the following query:\n\n{query_str}\n\n") |
There was a problem hiding this comment.
Why rethrowing the same exception with a different error message? Is the original error message not clear enough? I think it's also possible to chain the original exception with a custom one but I don't remember the syntax
There was a problem hiding this comment.
It is helpful to see the query that was being executed. In that way users can test it externally (for example here: http://archive.eso.org/programmatic/#TAP) and pinpoint the source of error. I found this to be the simplest way of achieving that goal, but I'm open to suggestions, if there's a better or standard way to do it.
There was a problem hiding this comment.
One could wonder if this shouldn't be addressed in TAP itself -> after all TAP could return the query as part of the error message.
Addresses #3138
Changes:
list_surveyslist_collectionsquery_surveysquery_collections*_surveys(...)in favor of_*collections(...)