Repository: astropy/pyvo

Issue:
getdatalink() exits messily when there isn't a datalink
Most simply, 

> import pyvo as vo
> import astropy.coordinates as coord
> coords = coord.SkyCoord.from_name("m51")
> skyview=vo.regsearch(servicetype='sia',keywords=['skyview digitized sky survey'])[0]
> results=skyview.search(pos=coords,radius=0.1)
> results[0].getdatalink()

This results in an expected exception because skyview doesn't offer datalinks with its results:
> DALServiceError: No Adhoc Service with ivo-id ivo://ivoa.net/std/datalink!

but then there's another exception because getdatalink() is doing something I don't understand.  Here's the code in adhoc.py around line 243:
```
    def getdatalink(self):
        try:
            datalink = self._results.get_adhocservice_by_ivoid(DATALINK_IVOID)

            query = DatalinkQuery.from_resource(self, datalink, session=self._session)
            return query.execute()
        except DALServiceError:
            return DatalinkResults.from_result_url(self.getdataurl(), session=self._session)
```
The problem is that the service is returning a URL to a FITS file.  When the get_adhocservice_by_ivoid() fails to find a datalink in the metadata of the result, it excepts into the call to DatalinkResults.from_result_url().  The problem in this case is that the data URL of this result is to a FITS file, and it's expecting a VOTable result, and it barfs trying to parse the FITS file as a VOTable.  

At first I thought that there should be a check for the current content_type, e.g., only call that when it's XML or VOTable or something.  But I don't understand what the use case is for this anyway, so I'm not sure what fix to propose.  

Personally, I think that if there is no datalink, the call to getdatalink() should return None rather than an exception.  


Discussion:
bsipocz: > squash-and-merge

That really messes up my release scripts, so would prefer if a PR is being clean on it's own. So if not a full-on rebase, then do an interactive rebase on the same base commit and squash out the iterations as you suggested Markus.

OTOH, I really would prefer not to have any of these ruff reformattings. If we will switch to use ruff, will have to do it on a separate gigantic windowdressing PR rather than as piecemeal hidden away in bugfix releases. But then clearly just go with what's in the file and fix the style of the newly added code. The CI job gives very clear instructions about what those 4 lines are, touching another 300+ is totally unnecessary:

```
 pyvo/dal/adhoc.py:350:1: W293 blank line contains whitespace
pyvo/dal/tests/test_datalink.py:34:1: E305 expected 2 blank lines after class or function definition, found 1
pyvo/dal/tests/test_datalink.py:53:1: E302 expected 2 blank lines, found 1
pyvo/dal/tests/test_datalink.py:340:29: W292 no newline at end of file
4
```

Patch:
diff --git a/CHANGES.rst b/CHANGES.rst
index 38f1f0e..639286d 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -38,6 +38,8 @@ Enhancements and Fixes
 
 - New sub-package discover for global dataset discovery. [#470]
 
+- Updated getdatalink to be consistent with iter_datalinks. [#613]
+
 
 Deprecations and Removals
 -------------------------
diff --git a/pyvo/dal/adhoc.py b/pyvo/dal/adhoc.py
index 54940c4..78c55cd 100644
--- a/pyvo/dal/adhoc.py
+++ b/pyvo/dal/adhoc.py
@@ -263,6 +263,15 @@ class DatalinkResultsMixin(AdhocServiceResultsMixin):
         if access_url:
             return access_url
 
+    @staticmethod
+    def _guess_datalink(row, **kwargs):
+        # TODO: we should be more careful about whitespace, case
+        # and perhaps more parameters in the following comparison
+        if row._results._guess_access_format(row) == DATALINK_MIME_TYPE:
+            access_url = row._results._guess_access_url(row)
+            if access_url is not None:
+                return DatalinkResults.from_result_url(access_url, **kwargs)
+
     def _iter_datalinks_from_product_rows(self):
         """yield datalinks from self's rows if they describe datalink-valued
         products.
@@ -309,13 +318,31 @@ class DatalinkRecordMixin:
     """
 
     def getdatalink(self):
+        """
+        Retrieve the datalink information for this record.
+
+        Returns
+        -------
+        DatalinkResults
+            The datalink results for this record.
+
+        Raises
+        ------
+        DALServiceError
+            If no datalink information is found for this record.
+        """
         try:
             datalink = self._results.get_adhocservice_by_ivoid(DATALINK_IVOID)
 
             query = DatalinkQuery.from_resource(self, datalink, session=self._session)
             return query.execute()
-        except DALServiceError:
-            return DatalinkResults.from_result_url(self.getdataurl(), session=self._session)
+        except DALServiceError as error:
+            datalink = self._results._guess_datalink(self, session=self._session)
+            if datalink is not None:
+                return datalink
+            else:
+                # re-raise the original error if nothing works
+                raise DALServiceError("No datalink found for record.") from error
 
     @stream_decode_content
     def getdataset(self, timeout=None):


Test patch:
diff --git a/pyvo/dal/tests/test_datalink.py b/pyvo/dal/tests/test_datalink.py
index 323a518..8c314c5 100644
--- a/pyvo/dal/tests/test_datalink.py
+++ b/pyvo/dal/tests/test_datalink.py
@@ -4,14 +4,16 @@
 Tests for pyvo.dal.datalink
 """
 from functools import partial
+import re
 
 import pytest
 
 import pyvo as vo
-from pyvo.dal.adhoc import DatalinkResults
+from pyvo.dal.adhoc import DatalinkResults, DALServiceError
 from pyvo.dal.sia2 import SIA2Results
 from pyvo.dal.tap import TAPResults
 from pyvo.utils import testing, vocabularies
+from pyvo.dal.sia import search
 
 from astropy.utils.data import get_pkg_data_contents, get_pkg_data_filename
 
@@ -30,6 +32,27 @@ def ssa_datalink(mocker):
         yield matcher
 
 
+sia_re = re.compile("http://example.com/sia.*")
+
+
+@pytest.fixture()
+def register_mocks(mocker):
+    with mocker.register_uri(
+        "GET",
+        "http://example.com/querydata/image.fits",
+        content=get_pkg_data_contents("data/querydata/image.fits"),
+    ) as matcher:
+        yield matcher
+
+
+@pytest.fixture()
+def sia(mocker):
+    with mocker.register_uri(
+        "GET", sia_re, content=get_pkg_data_contents("data/sia/dataset.xml")
+    ) as matcher:
+        yield matcher
+
+
 @pytest.fixture()
 def datalink(mocker):
     def callback(request, context):
@@ -307,3 +330,14 @@ class TestIterDatalinksProducts:
         assert len(links) == 1
         assert (next(links[0].bysemantics("#this"))["access_url"]
             == "http://dc.zah.uni-heidelberg.de/getproduct/flashheros/data/ca90/f0011.mt")
+
+
+@pytest.mark.usefixtures("sia", "register_mocks")
+@pytest.mark.filterwarnings("ignore::astropy.io.votable.exceptions.W06")
+def test_no_datalink():
+    # for issue #328 getdatalink() exits messily when there isn't a datalink
+
+    results = search("http://example.com/sia", pos=(288, 15), format="all")
+    result = results[0]
+    with pytest.raises(DALServiceError, match="No datalink found for record."):
+        result.getdatalink()