Repository: astropy/pyvo
Pull request #241 (2020-09-30 12:14:20)

Make datalink .bysemantics semantics-aware
This will fix bug #231.

Specifically, this PR adds utils.vocabularies, which is usable beyond
datalink to pull IVOA vocabularies as specified by the (current)
Vocabularies in the VO 2 WD (I will update the module in case there
are major changes to the WD),
http://ivoa.net/documents/Vocabularies/20200612/ .

Using this, it extends DatalinkResults.bysemantics to accept lists as
well as strings (as before).  It also accepts "naked" terms now (i.e.,
without the hash that is a bit of a serialisation hack in datalink).
I'm not attempting to recognise full resource URIs at this point (but
that would be easy and safe to add once someone actually has a use for that).

Most importantly, as long as include_narrower is True (the default),
this consults the datalink vocabulary and will match narrower terms as
well; for instance, just looking for calibration will give you dark, bias, and flat as
well.

andamian: @msdemlei - We've reworked the CI. Please pull the master to see if that fixes the tests so that we can proceed with the next steps. And sorry for the delay.
msdemlei: On Wed, Dec 09, 2020 at 08:00:27AM -0800, andamian wrote:
> @msdemlei - We've reworked the CI. Please pull the master to see if that fixes the tests so that we can proceed with the next steps. And sorry for the delay.

Hm -- regrettably not quite.

The changelog entry I've added (but not pushed yet for environmental
concerns).  The failures with astropy 4.1 I've not quite figured out
yet, mainly because I got lost in the pytest version hell.  With
pytest 6.1.2 in the virtual environment (on top of Debian 3.7) the
thing consistently throws

  cannot import name 'transfer_markers' from '_pytest.python'

when trying to run any tests.  Before I delve into this any deeper:
Is there a known solution for this?

andamian: @msdemlei I took the liberty of fixing the tests. Small change - they are now passing. Please pull.

Sorry, I'm not familiar with the `pytest` problem on `Debian`. If you are familiar with `tox`, you can let `tox` deal with virtual environment setup. All the tests in CI rely on that, so if it's something wrong with it they can all benefit.

Feel free to push the change long to this branch. That should clear the other remaining error.

Sorry for the duplicates in the tests - the CI framework was created mainly for PR from forks and I'm not quite sure how to fix it when the PR originates from the master repo.
codecov[bot]: # [Codecov](https://codecov.io/gh/astropy/pyvo/pull/241?src=pr&el=h1) Report
> Merging [#241](https://codecov.io/gh/astropy/pyvo/pull/241?src=pr&el=desc) (b117b26) into [master](https://codecov.io/gh/astropy/pyvo/commit/062710b30f21d330933856ae0d9b9592e9f1de8d?el=desc) (062710b) will **decrease** coverage by `0.07%`.
> The diff coverage is `62.96%`.

[![Impacted file tree graph](https://codecov.io/gh/astropy/pyvo/pull/241/graphs/tree.svg?width=650&height=150&src=pr&token=Mynyo9xoPZ)](https://codecov.io/gh/astropy/pyvo/pull/241?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #241      +/-   ##
==========================================
- Coverage   74.83%   74.75%   -0.08%     
==========================================
  Files          43       44       +1     
  Lines        5035     5087      +52     
==========================================
+ Hits         3768     3803      +35     
- Misses       1267     1284      +17     
```


| [Impacted Files](https://codecov.io/gh/astropy/pyvo/pull/241?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pyvo/utils/vocabularies.py](https://codecov.io/gh/astropy/pyvo/pull/241/diff?src=pr&el=tree#diff-cHl2by91dGlscy92b2NhYnVsYXJpZXMucHk=) | `50.00% <50.00%> (ø)` | |
| [pyvo/dal/adhoc.py](https://codecov.io/gh/astropy/pyvo/pull/241/diff?src=pr&el=tree#diff-cHl2by9kYWwvYWRob2MucHk=) | `66.10% <100.00%> (+1.74%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/astropy/pyvo/pull/241?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/astropy/pyvo/pull/241?src=pr&el=footer). Last update [062710b...8d950b2](https://codecov.io/gh/astropy/pyvo/pull/241?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

msdemlei: Can I bump this?  I'm not really sure if the failed codecov tests are something I should be trying to fix of if that's just CI noise...
andamian: The report says that the coverage decreased slightly. If you can bump the coverage for your patch from 62% to 75%, it will pass. If you follow the `Details` link in `codecov/patch` it highlights the new code that is not tested. Maybe you could add a test or two to push it over the 75% and turn everything green. The way is set right now, it doesn't look like GH allows merging otherwise.
msdemlei: On Thu, Mar 04, 2021 at 09:44:22AM -0800, andamian wrote:
> The report says that the coverage decreased slightly. If you can
> bump the coverage for your patch from 62% to 75%, it will pass. If
> you follow the `Details` link in `codecov/patch` it highlights the
> new code that is not tested. Maybe you could add a test or two to
> push it over the 75% and turn everything green. The way is set
> right now, it doesn't look like GH allows merging otherwise.

Regrettably, the codecov links all give me "403 unauthorised", and my
patience for debugging and working around crazy auth schemes "in the
cloud" is relatively low, in particular where codecov.io's docs don't
work unless you let them execute Javascript (WTF?).

But I'd guess the reason coverage looks so bad is that
utils.vocabularies has the conditional code for astrpy versions <4
that won't be exercised in version 4 because it simply doesn't run in
that version.

It turns out codecov use coverage.py, and so I'm escaping from this
by just using a pragma to keep the compatibility code out of
coverage.py's view.  I'd say that's fair, even more so since we can
perhaps drop the compatibility code soon-ish anyway (though I'm
always a big fan of maintaining backwards compatiblity as long as we
can).

After that, the codecov is fine with the change.


andamian: I'm not sure why that info is not accessible. Maybe @pllim might have an idea.

The coverage report is also available from the "Checks" tab as GH code comments.

Is the PR now ready for code review? Do you have a suggestion for a reviewer?
pllim: Hello. Exactly which link gave you a 403? Can you please clarify?

I know CircleCI now requires login to see the results but I am not sure about Codecov.
msdemlei: On Fri, Mar 05, 2021 at 09:41:11AM -0800, andamian wrote:
> I'm not sure why that info is not accessible. Maybe @pllim might have an idea.

Hm... the equivalent links the github PR page now shows work.
Perhaps the previous results were just expired?

> Is the PR now ready for code review? Do you have a suggestion for a reviewer?

I'd like to think it could be reviewed, but I'm afraid I can't
suggest a reviewer.  On the other hand, I'd say the code as such by
and large is straightforward, so no special skills are required
there.  As to the feature added itself, perhaps the original issue
helps giving the larger picture?

andamian: In this case, I'll do the code review. @cbanek or @funbaker or anyone else interested please feel free to jump in.

funbaker: Gonna try to have a look at the weekend.
msdemlei: On Fri, Mar 12, 2021 at 04:09:57PM -0800, andamian wrote:
> +def get_label(voc, term):
> +    """returns the label of term if it's in the desise vocabulary voc,
> +    term capitalised otherwise.
> +    """
> +    if term in voc["terms"]:
> +        return voc["terms"][term]["label"]
> +    else:
> +        return term.capitalize()
> 
> Why capitalize? Probably something that I'm not familiar with in
> dictionaries. Also, shouldn't this trigger a refresh of the cache
> or it's not worth it?

The capitalisation is just a matter of taste and could be dropped --
on the other hand, human-readable labels should generally be
capitalised in the vocabularies, and doing it for the occasional
out-of-vocabulary term probably improves general apprearance.  Still,
I'll drop it if you think it's unneccessary frills.

On refreshing: I'm pretty sure a cache miss shouldn't trigger
updates, as cache misses could come with high frequency; you'd then
have to have some sort of rate limiting ("only re-try after N
hours"), and at least at this point this seems a lot more
complication than it might one day help.

Generally, I'd say any auto-updating policies should be developed as
we gain experience with how vocabularies get to be used.

andamian: > On Fri, Mar 12, 2021 at 04:09:57PM -0800, andamian wrote: +def get_label(voc, term): + """returns the label of term if it's in the desise vocabulary voc, + term capitalised otherwise. + """ + if term in voc["terms"]: + return voc["terms"][term]["label"] + else: + return term.capitalize() Why capitalize? Probably something that I'm not familiar with in dictionaries. Also, shouldn't this trigger a refresh of the cache or it's not worth it?
> The capitalisation is just a matter of taste and could be dropped -- on the other hand, human-readable labels should generally be capitalised in the vocabularies, and doing it for the occasional out-of-vocabulary term probably improves general apprearance. Still, I'll drop it if you think it's unneccessary frills. On refreshing: I'm pretty sure a cache miss shouldn't trigger updates, as cache misses could come with high frequency; you'd then have to have some sort of rate limiting ("only re-try after N hours"), and at least at this point this seems a lot more complication than it might one day help. Generally, I'd say any auto-updating policies should be developed as we gain experience with how vocabularies get to be used.

If capitalization is desired, should we try to do it even for the labels of existing terms? That aside, I would suggest adding a `default` argument to the method that is returned when the term no not found. That would be consistent with dictionary functionality.

OK on refreshing.

msdemlei: On Mon, Mar 15, 2021 at 02:47:37PM -0700, andamian wrote:
> If capitalization is desired, should we try to do it even for the
> labels of existing terms? 

No -- the vocabulary designers may have good reasons for using lower
case in labels (think of astronomical nomenclature that is often
case-sensitive).  On the other hand, when people invent terms,
there's probably little thought involved :-).  But as I said: if
you're worried, I'll remove it.

> That aside, I would suggest adding a `default` argument to the
> method that is returned when the term no not found. That would be
> consistent with dictionary functionality.

That is a good point.  I've put it in in 3749b7c2.

msdemlei: On Wed, Mar 17, 2021 at 08:45:16PM -0700, andamian wrote:
> @andamian commented on this pull request.
> 
> 
> 
> +    else:
> +        return default if default is not None else term.capitalize()
> 
> I would go further and just return `default`. Especially if default
> capitalized term is more a matter of taste as you said. Right now
> it hides the case when the label does not exist. But I'm fine with
> this too.

You're right.  One less selection is almost always a win, and it
was a questionable functionality in the first place.

I was briefly considering whether there's a point in making raising a
KeyError the default, but if people want that, they can directly look
in voc.terms.

msdemlei: On Mon, Mar 22, 2021 at 06:43:39AM -0700, andamian wrote:
> @andamian commented on this pull request.
> +if astropy.__version__<'4': # pragma: no cover
> 
> @msdemlei have you tried to remove this `if` statement?

Can I pull a Bartleby ("I would prefer not to")?

You see, I'm running this on Debian stable, and I would love it if
people on buster could use this without having to upgrade astropy
(which is a bit of a hassle).

However, if it bothers you it can obviously go -- it clearly doesn't
have any effect on current astropys.

andamian: > On Mon, Mar 22, 2021 at 06:43:39AM -0700, andamian wrote: @andamian commented on this pull request. +if astropy.__version__<'4': # pragma: no cover @msdemlei have you tried to remove this `if` statement?
> Can I pull a Bartleby ("I would prefer not to")? You see, I'm running this on Debian stable, and I would love it if people on buster could use this without having to upgrade astropy (which is a bit of a hassle). However, if it bothers you it can obviously go -- it clearly doesn't have any effect on current astropys.

This would be code that we don't test during the CI but we'll have to support if we release. It's true that we don't have a lower limit on `astropy` dependencies yet which I think we should. If we add that limit, then clients won't be able to install the latest `pyvo` without upgrading `astropy` and this will become dead code.
andamian: @msdemlei thanks for making the changes. I've approved the PR. @funbaker - are you still planing to review it? If not, we'll merge it into master.

Patch:
diff --git a/.github/workflows/cibuild.yml b/.github/workflows/cibuild.yml
index e904641..98ea83b 100644
--- a/.github/workflows/cibuild.yml
+++ b/.github/workflows/cibuild.yml
@@ -40,7 +40,11 @@ jobs:
     - name: Install tox
       run: python -m pip install --upgrade tox
     - name: Python ${{ matrix.python-version }} with astropy
-      run: tox -e py${{ matrix.python-version }}-${{ matrix.astropy-version }}
+      run: |
+        # remove "." from matrix.python-version to get the defaults tox python versions
+        tox_python_target=$(echo py${{matrix.python-version}} | sed 's/\.//')
+        echo "Tox python target: " $tox_python_target
+        tox -e $tox_python_target-${{ matrix.astropy-version }}
 
 
   mac_windows:
@@ -62,7 +66,7 @@ jobs:
     - name: Install tox
       run: python -m pip install --upgrade tox
     - name: Python 3.8 with latest astropy
-      run: tox -e py3.8-latest
+      run: tox -e py38-latest
 
 
   style_coverage:
@@ -101,7 +105,7 @@ jobs:
       - name: Install tox
         run: python -m pip install --upgrade tox
       - name: Run tests against astropy dev
-        run: tox -e py3.8-astropydev
+        run: tox -e py38-astropydev
 
   #publish:
   #needs: coverage
diff --git a/CHANGES.rst b/CHANGES.rst
index cf744cb..551d338 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -1,6 +1,8 @@
 1.2 (unreleased)
 ================
 
+- Make .bysemantics expand its terms to the entire branch by default [#241]
+
 
 1.1 (2020-06-26)
 ================
diff --git a/pyvo/dal/adhoc.py b/pyvo/dal/adhoc.py
index 388c690..ce5ca90 100644
--- a/pyvo/dal/adhoc.py
+++ b/pyvo/dal/adhoc.py
@@ -22,6 +22,7 @@ from astropy.io.votable.tree import Resource, Group
 from astropy.utils.collections import HomogeneousList
 
 from ..utils.decorators import stream_decode_content
+from ..utils import vocabularies
 from .params import PosQueryParam, IntervalQueryParam, TimeQueryParam,\
     EnumQueryParam
 from ..dam.obscore import POLARIZATION_STATES
@@ -516,18 +517,42 @@ class DatalinkResults(DatalinkResultsMixin, DALResults):
         """
         return DatalinkRecord(self, index, session=self._session)
 
-    def bysemantics(self, semantics):
+    def bysemantics(self, semantics, include_narrower=True):
         """
         return the rows with the dataset identified by the given semantics
 
+        Parameters
+        ----------
+        semantics: str or list
+            One or more term(s) from the datalink vocabulary
+            (http://www.ivoa.net/rdf/datalink/core).  Terms may be
+            passed in with or without a leading hash.  Note that at
+            this point non-datalink/core URIs (i.e., full resource
+            URIs) are not supported (and will be butchered).
+        include_narrower: boolean
+            If true, the result will include matches for any term
+            that is narrower than the term passed in.
+
         Returns
         -------
         Sequence of DatalinkRecord
             a sequence of dictionary-like wrappers containing the result record
         """
-        # TODO: get semantics with astropy and implement resursive lookup
+        if isinstance(semantics, str):
+            semantics = [semantics]
+        semantics = [term.lstrip("#") for term in semantics]
+
+        if include_narrower:
+            additional_terms = []
+            voc = vocabularies.get_vocabulary("datalink/core")
+            for term in semantics:
+                if term in voc["terms"]:
+                    additional_terms.extend(voc["terms"][term]["narrower"])
+            semantics = semantics+additional_terms
+
+        semantics = set("#"+term for term in semantics)
         for record in self:
-            if record.semantics == semantics:
+            if record.semantics in semantics:
                 yield record
 
     def clone_byid(self, id):
diff --git a/pyvo/utils/vocabularies.py b/pyvo/utils/vocabularies.py
new file mode 100644
index 0000000..c8b1c6e
--- /dev/null
+++ b/pyvo/utils/vocabularies.py
@@ -0,0 +1,74 @@
+"""
+A shallow interface to IVOA vocabularies.
+
+See http://ivoa.net/documents/Vocabularies/ (>= version 2) for the
+larger background.  In this module, we essentially wrap the retrieval
+and caching of the desise files.
+"""
+
+import functools
+import json
+import os
+import time
+from urllib import request
+
+from astropy.utils.data import download_file, clear_download_cache
+
+IVOA_VOCABULARY_ROOT = "http://www.ivoa.net/rdf/"
+
+
+class VocabularyError(Exception):
+    """A generic error that occurred when interacting with the IVOA
+    vocabulary repository.
+    """
+
+
+@functools.lru_cache()
+def get_vocabulary(voc_name, force_update=False):
+    """returns an IVOA vocabulary in its "desise" form.
+
+    See Vocabularies in the VO 2 to see what is inside of this.
+
+    This will use a cache to avoid repeated updates, but it
+    will attempt to re-download if the cached copy is older than 6 months.
+    """
+    src_url = IVOA_VOCABULARY_ROOT+voc_name
+    if force_update:
+        clear_download_cache(src_url)
+
+    try:
+        src_name = download_file(
+            src_url,
+            cache=True,
+            show_progress=False,
+            http_headers={"accept": "application/x-desise+json"})
+    except Exception as msg:
+        raise VocabularyError("No such vocabulary: {} ({})".format(
+            voc_name, msg))
+
+    if time.time()-os.path.getmtime(src_name)>3600*60*150:
+        # attempt a re-retrieval, but ignore failure
+        try:
+            src_name = download_file(
+                IVOA_VOCABULARY_ROOT+voc_name,
+                cache="update", show_progress=False,
+                http_headers={"accept": "application/x-desise+json"})
+        except Exception as msg:
+            base.ui.notifyWarning("Updating cache for the vocabulary"
+                " {} failed: {}".format(voc_name, msg))
+
+    with open(src_name, "r", encoding="utf-8") as f:
+        return json.load(f)
+
+
+def get_label(voc, term, default=None):
+    """returns the label of term if it's in the desise vocabulary voc,
+    term capitalised otherwise.
+    """
+    if term in voc["terms"]:
+        return voc["terms"][term]["label"]
+    else:
+        return default
+
+
+# vi:et:sw=4:sta
diff --git a/tox.ini b/tox.ini
index 1c26416..2a7f5ef 100644
--- a/tox.ini
+++ b/tox.ini
@@ -1,7 +1,12 @@
 [tox]
 envlist =
-    py{3.7,3.8,3.9}-{latest,astropy40,astropy41}
-    py3.8-astropydev
+    py{37,38,39}-{latest,astropy40,astropy41}
+    py38-astropydev
+    egg_info
+    cov
+    linkcheck
+    codestyle
+
 requires =
     setuptools >= 30.3.0
     pip >= 19.3.1


Test patch:
diff --git a/pyvo/dal/tests/data/datalink/proc.xml b/pyvo/dal/tests/data/datalink/proc.xml
index 453e47a..78e1c60 100644
--- a/pyvo/dal/tests/data/datalink/proc.xml
+++ b/pyvo/dal/tests/data/datalink/proc.xml
@@ -57,6 +57,16 @@
             <TD/>
             <TD/>
             <TD>A preview for the dataset.</TD>
+            <TD>#preview-image</TD>
+            <TD>image/jpeg</TD>
+            <TD>-1</TD>
+          </TR>
+          <TR>
+            <TD>ivo://org.gavo.dc/s/data/gds_big/v6a/2010/GDS_0644-0035/i_s/eq010000ms/20100927.comb_avg.0001.fits.fz</TD>
+            <TD>http://dc.zah.uni-heidelberg.de/wider.dat</TD>
+            <TD/>
+            <TD/>
+            <TD>An artificial thing.</TD>
             <TD>#preview</TD>
             <TD>image/jpeg</TD>
             <TD>-1</TD>
@@ -128,7 +138,7 @@
           <OPTION name="Retrieve the full data, including header (default)" value="DATA"/>
         </VALUES>
       </PARAM>
-      <PARAM ID="BAND" arraysize="2" datatype="double" name="BAND" ucd="em.wl" unit="m" value="[0. 0.]" xtype="interval"/>
+      <PARAM ID="BAND" arraysize="2" datatype="double" name="BAND" ucd="em.wl" unit="m" xtype="interval"/>
     </GROUP>
     <PARAM arraysize="*" datatype="char" name="accessURL" ucd="meta.ref.url" value="http://example.com/proc_ds"/>
     <PARAM arraysize="*" datatype="char" name="standardID" value="ivo://ivoa.net/std/SODA#sync-1.0"/>
diff --git a/pyvo/dal/tests/test_datalink.py b/pyvo/dal/tests/test_datalink.py
index c771974..f22e82a 100644
--- a/pyvo/dal/tests/test_datalink.py
+++ b/pyvo/dal/tests/test_datalink.py
@@ -8,6 +8,7 @@ from functools import partial
 import pytest
 
 import pyvo as vo
+from pyvo.dal.adhoc import DatalinkResults
 
 from astropy.utils.data import get_pkg_data_contents
 
@@ -67,6 +68,17 @@ def res_datalink(mocker):
         yield matcher
 
 
+@pytest.fixture()
+def proc(mocker):
+    def callback(request, context):
+        return get_pkg_data_contents('data/datalink/proc.xml')
+
+    with mocker.register_uri(
+        'GET', 'http://example.com/proc', content=callback
+    ) as matcher:
+        yield matcher
+
+
 @pytest.mark.usefixtures('ssa_datalink', 'datalink')
 @pytest.mark.filterwarnings("ignore::astropy.io.votable.exceptions.W27")
 @pytest.mark.filterwarnings("ignore::astropy.io.votable.exceptions.W06")
@@ -101,3 +113,52 @@ def test_datalink_batch():
         'http://example.com/obscore', (30, 30))
 
     assert len([_ for _ in results.iter_datalinks()]) == 3
+
+
+# temporary hack while astropy <4.1 is still around; you can remove
+# this and its calls once that's no longer true.
+def _debytify(v):
+    if isinstance(v, bytes):
+        return v.decode("utf-8")
+    return v
+
+
+@pytest.mark.usefixtures('proc')
+@pytest.mark.filterwarnings("ignore::astropy.io.votable.exceptions.W27")
+@pytest.mark.filterwarnings("ignore::astropy.io.votable.exceptions.W06")
+@pytest.mark.filterwarnings("ignore::astropy.io.votable.exceptions.W48")
+@pytest.mark.filterwarnings("ignore::astropy.io.votable.exceptions.E02")
+class TestSemanticsRetrieval:
+    def test_access_with_string(self):
+        datalink = DatalinkResults.from_result_url('http://example.com/proc')
+        res = [_debytify(r["access_url"])
+            for r in datalink.bysemantics("#this")]
+        assert len(res)==1
+        assert res[0].endswith("eq010000ms/20100927.comb_avg.0001.fits.fz")
+
+    def test_access_with_list(self):
+        datalink = DatalinkResults.from_result_url('http://example.com/proc')
+        res = [_debytify(r["access_url"])
+            for r in datalink.bysemantics(["#this", "#preview-image"])]
+        assert len(res)==2
+        assert res[0].endswith("eq010000ms/20100927.comb_avg.0001.fits.fz")
+        assert res[1].endswith("20100927.comb_avg.0001.fits.fz?preview=True")
+
+    def test_access_with_expansion(self):
+        datalink = DatalinkResults.from_result_url('http://example.com/proc')
+        res = [_debytify(r["access_url"])
+            for r in datalink.bysemantics(["#this", "#preview"])]
+        assert len(res)==3
+        assert res[0].endswith("eq010000ms/20100927.comb_avg.0001.fits.fz")
+        assert res[1].endswith("20100927.comb_avg.0001.fits.fz?preview=True")
+        assert res[2].endswith("http://dc.zah.uni-heidelberg.de/wider.dat")
+
+    def test_access_without_expansion(self):
+        datalink = DatalinkResults.from_result_url('http://example.com/proc')
+        res = [_debytify(r["access_url"])
+            for r in datalink.bysemantics(
+               ["#this", "#preview"],
+               include_narrower=False)]
+        assert len(res)==2
+        assert res[0].endswith("eq010000ms/20100927.comb_avg.0001.fits.fz")
+        assert res[1].endswith("http://dc.zah.uni-heidelberg.de/wider.dat")
diff --git a/pyvo/utils/tests/test_vocabularies_remote.py b/pyvo/utils/tests/test_vocabularies_remote.py
new file mode 100644
index 0000000..3c88256
--- /dev/null
+++ b/pyvo/utils/tests/test_vocabularies_remote.py
@@ -0,0 +1,55 @@
+# Licensed under a 3-clause BSD style license - see LICENSE.rst
+"""
+Tests for pyvo.utils.vocabularies
+
+It's hard to write meaningful tests for those that don't require network
+connectivity because essentially it's all just wrapping downloads.  Hence,
+I'm just giving in rather than bother with a mock server.
+"""
+
+import os
+import time
+
+import pytest
+
+from astropy.utils import data
+
+from pyvo.utils import vocabularies
+
+
+@pytest.mark.remote_data
+class TestVocabularies:
+
+    def test_basic_getting(self):
+        voc = vocabularies.get_vocabulary("datalink/core")
+        assert "progenitor" in voc["terms"]
+        assert data.is_url_in_cache("http://www.ivoa.net/rdf/datalink/core")
+
+    def test_label_getting(self):
+        voc = vocabularies.get_vocabulary("datalink/core")
+        assert (vocabularies.get_label(voc, "coderived")
+            == "Coderived Data")
+
+    def test_label_getting_default(self):
+        voc = vocabularies.get_vocabulary("datalink/core")
+        assert vocabularies.get_label(voc, "oov", "Missing") == "Missing"
+
+    def test_refreshing(self):
+        voc = vocabularies.get_vocabulary("datalink/core", force_update=True)
+
+        # first make sure that things didn't break
+        assert "progenitor" in voc["terms"]
+
+        # now guess that a download has actually happened; we don't want
+        # to reflect cache name generation here, so we just check if there's
+        # a recent download in the cache directory
+        dldir = data._get_download_cache_loc()
+        with os.scandir(dldir) as entries:
+            last_change = 0
+            for entry in entries:
+                last_change = max(last_change, entry.stat().st_mtime)
+        assert time.time()-last_change<2
+
+    def test_non_existing_voc(self):
+        with pytest.raises(vocabularies.VocabularyError):
+            vocabularies.get_vocabulary("not_an_ivoa_vocabulary")