From a5b25cb51b20e8a169373582df985ee3ae962711 Mon Sep 17 00:00:00 2001 From: Laure Thompson <602628+laurejt@users.noreply.github.com> Date: Fri, 20 Mar 2026 16:23:45 -0400 Subject: [PATCH 01/13] Add ruff support and ruff pre-commit hook. In addition to superficial changes, ruff did catch a small error in a unit test. For now, not including any ruff rules on type annotations or docstrings. --- .pre-commit-config.yaml | 14 ++++ ci/testsettings.py | 11 ++- pyproject.toml | 24 +++++- sphinx-docs/conf.py | 15 ++-- sphinx-docs/docsettings.py | 13 ++- src/viapy/api.py | 26 +++--- src/viapy/test_urls.py | 6 +- src/viapy/urls.py | 1 - src/viapy/views.py | 77 ++++++++++-------- src/viapy/widgets.py | 5 +- tests/test_api.py | 42 +++++----- tests/test_django.py | 158 ++++++++++++++++++------------------- uv.lock | 27 +++++++ 13 files changed, 248 insertions(+), 171 deletions(-) create mode 100644 .pre-commit-config.yaml diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 0000000..e8c5cdd --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,14 @@ +# This config file specifies which pre-commit hooks to run. +# +# To set up, run `pre-commit install` +# To run all hooks manually, run `pre-commit run --all-files` +repos: + # Ruff Python linter and formatter (ruff configs in pyproject.toml) + - repo: https://github.com/astral-sh/ruff-pre-commit + rev: v0.12.7 + hooks: + # Run the linter + - id: ruff-check + args: [--fix, --show-fixes] # enable lint fixes + # Run the formatter + - id: ruff-format diff --git a/ci/testsettings.py b/ci/testsettings.py index 96219f7..8edd698 100644 --- a/ci/testsettings.py +++ b/ci/testsettings.py @@ -8,15 +8,14 @@ INSTALLED_APPS = ( # 'django.contrib.contenttypes', - 'dal', - 'dal_select2', - 'viapy', + "dal", + "dal_select2", + "viapy", ) -ROOT_URLCONF = 'viapy.test_urls' +ROOT_URLCONF = "viapy.test_urls" -LANGUAGE_CODE = 'en-us' +LANGUAGE_CODE = "en-us" # must be added # SECRET_KEY = '' - diff --git a/pyproject.toml b/pyproject.toml index a616891..26ed823 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -57,7 +57,13 @@ django = ["django>=3.2", "django-autocomplete-light"] django_test = ["pytest-django", "viapy[django]"] docs = ["sphinx"] test_all = ["viapy[test]", "viapy[django_test]"] -dev = ["pre-commit", "viapy[django]", "viapy[test_all]", "viapy[docs]"] +dev = [ + "ruff", + "pre-commit", + "viapy[django]", + "viapy[test_all]", + "viapy[docs]" +] [dependency-groups] dev = ["viapy[dev]"] @@ -80,3 +86,19 @@ exclude_lines = [ [tool.coverage.html] directory = "coverage_html_report" + +[tool.ruff.lint] +# Include these rules in addition to ruff's defaults +extend-select = [ + "B", # flake8-bugbear + "C4", # flake8-comprehensions + "I", #isort + "NPY", # numpy-specific rules + "PERF", # perflint + "PTH", # flake8-use-pathlib + "RUF", # ruff-specific rules + "SIM", # flake8-simplify + "UP", # pyupgrade +] +# Can use to ignore specific rules within above selection +ignore = [] diff --git a/sphinx-docs/conf.py b/sphinx-docs/conf.py index 05de731..1565c4e 100644 --- a/sphinx-docs/conf.py +++ b/sphinx-docs/conf.py @@ -1,5 +1,4 @@ #!/usr/bin/env python3 -# -*- coding: utf-8 -*- # # viapy documentation build configuration file, created by # sphinx-quickstart on Thu Oct 12 17:06:23 2017. @@ -15,23 +14,25 @@ # If extensions (or modules to document with autodoc) are in another directory, # add these directories to sys.path here. If the directory is relative to the -# documentation root, use os.path.abspath to make it absolute, like shown here. +# documentation root, use pathlib.Path.resolve to make it absolute, like shown here. # -# import os +# import pathlib # import sys -# sys.path.insert(0, os.path.abspath('.')) +# sys.path.insert(0, pathlib.Path().resolve()) import os +import pathlib import sys + import django -sys.path.insert(0, os.path.abspath(".")) +from viapy import __version__ + +sys.path.insert(0, pathlib.Path().resolve()) os.environ["DJANGO_SETTINGS_MODULE"] = "docsettings" django.setup() -from viapy import __version__ - # -- General configuration ------------------------------------------------ diff --git a/sphinx-docs/docsettings.py b/sphinx-docs/docsettings.py index fbc5e9c..41fab1e 100644 --- a/sphinx-docs/docsettings.py +++ b/sphinx-docs/docsettings.py @@ -8,14 +8,13 @@ INSTALLED_APPS = ( # 'django.contrib.contenttypes', - 'dal', - 'dal_select2', - 'viapy', + "dal", + "dal_select2", + "viapy", ) -ROOT_URLCONF = 'viapy.test_urls' +ROOT_URLCONF = "viapy.test_urls" -LANGUAGE_CODE = 'en-us' - -SECRET_KEY = 'f$J4FB3B=}1bFtJ$}b9s28Vsf?&otV}o0*V)g;#OD5%20uksel' +LANGUAGE_CODE = "en-us" +SECRET_KEY = "f$J4FB3B=}1bFtJ$}b9s28Vsf?&otV}o0*V)g;#OD5%20uksel" diff --git a/src/viapy/api.py b/src/viapy/api.py index 875abb6..b73658e 100644 --- a/src/viapy/api.py +++ b/src/viapy/api.py @@ -2,20 +2,19 @@ import re import time +import rdflib +import requests from attrdict import AttrMap from cached_property import cached_property -import requests -import rdflib from rdflib.namespace import Namespace - logger = logging.getLogger(__name__) SCHEMA_NS = Namespace("http://schema.org/") -class ViafAPI(object): +class ViafAPI: """Wrapper for VIAF API. https://platform.worldcat.org/api-explorer/apis/VIAF @@ -31,14 +30,14 @@ class ViafAPI(object): @classmethod def uri_from_id(cls, viaf_id): """Generate a canonical VIAF URI for the specified id""" - return "%s/%s" % (cls.uri_base, viaf_id) + return f"{cls.uri_base}/{viaf_id}" def suggest(self, term): """Query autosuggest API. Returns a list of results, or an empty list if no suggestions are found or if something went wrong""" # 'viaf/AutoSuggest?query=[searchTerms]&callback[optionalCallbackName] - autosuggest_url = "%s/AutoSuggest" % self.api_base + autosuggest_url = f"{self.api_base}/AutoSuggest" response = requests.get( autosuggest_url, params={"query": term}, @@ -64,7 +63,7 @@ def search(self, query): """Query VIAF seach interface. Returns a list of :class:`SRUItem` :param query: CQL query in viaf syntax (e.g., ``cql.any all "term"``) """ - search_url = "%s/search" % self.api_base + search_url = f"{self.api_base}/search" params = { "query": query, "maximumRecords": 10, # TODO: configurable ? @@ -73,7 +72,9 @@ def search(self, query): "sortKey": "holdingscount", } - response = requests.get(search_url, params=params, headers={"Accept": "application/json"}) + response = requests.get( + search_url, params=params, headers={"Accept": "application/json"} + ) logger.debug( "search '%s': %s %s, %0.2f", params["query"], @@ -90,7 +91,7 @@ def search(self, query): response.raise_for_status() def _find_type(self, fltr, value): - return self.search('%s all "%s"' % (fltr, value)) + return self.search(f'{fltr} all "{value}"') def find_person(self, name): """Search VIAF for local.personalNames""" @@ -105,7 +106,7 @@ def find_place(self, name): return self._find_type("local.geographicNames", name) -class ViafEntity(object): +class ViafEntity: """Object for working with a single VIAF entity. :param viaf_id: viaf identifier (either integer or uri) @@ -177,7 +178,7 @@ def year_from_isodate(cls, date): return value -class SRUResult(object): +class SRUResult: """SRU search result object, for use with :meth:`ViafAPI.search`.""" def __init__(self, data): @@ -213,10 +214,11 @@ def normalize_record(self, data): else: return data + class SRUItem(AttrMap): """Single item returned by a SRU search, for use with :meth:`ViafAPI.search` and :class:`SRUResult`. - + The `VIAFCluster` attribute was added to each property lookup in 2025 to match updates to the /search API's JSON response.""" diff --git a/src/viapy/test_urls.py b/src/viapy/test_urls.py index d5a3511..be494cb 100644 --- a/src/viapy/test_urls.py +++ b/src/viapy/test_urls.py @@ -1,7 +1,7 @@ -"""Test URL configuration for viapy -""" +"""Test URL configuration for viapy""" + try: - from django.urls import path, include + from django.urls import include, path from viapy import urls as viapy_urls diff --git a/src/viapy/urls.py b/src/viapy/urls.py index 8e35c1f..5f86b8b 100644 --- a/src/viapy/urls.py +++ b/src/viapy/urls.py @@ -2,7 +2,6 @@ from viapy.views import ViafLookup, ViafSearch - app_name = "viapy" urlpatterns = [ diff --git a/src/viapy/views.py b/src/viapy/views.py index a016e30..c648168 100644 --- a/src/viapy/views.py +++ b/src/viapy/views.py @@ -1,15 +1,15 @@ -from django.http import JsonResponse from dal import autocomplete +from django.http import JsonResponse from viapy.api import ViafAPI class ViafLookup(autocomplete.Select2ListView): - '''View to provide VIAF suggestions for autocomplete lookup. + """View to provide VIAF suggestions for autocomplete lookup. Based on :class:`dal.autocompleteSelect2ListView`. Expects search term as query string parameter `q`. Returns viaf URI as identifier and display form as text. - ''' + """ def get(self, request, *args, **kwargs): """Return JSON with suggested VIAF ids and display names.""" @@ -17,53 +17,64 @@ def get(self, request, *args, **kwargs): result = viaf.suggest(self.q) # optionally filter by nametype if set - if 'nametype' in self.kwargs: - result = [item for item in result - if item['nametype'] == self.kwargs['nametype']] + if "nametype" in self.kwargs: + result = [ + item for item in result if item["nametype"] == self.kwargs["nametype"] + ] - return JsonResponse({ - 'results': [dict( - id=viaf.uri_from_id(item['viafid']), - id_number=item['viafid'], - text=item['displayForm'], - nametype=item['nametype'] - # exclude any names that are not personal - ) for item in result] - }) + return JsonResponse( + { + "results": [ + { + "id": viaf.uri_from_id(item["viafid"]), + "id_number": item["viafid"], + "text": item["displayForm"], + "nametype": item["nametype"], + # exclude any names that are not personal + } + for item in result + ] + } + ) class ViafSearch(autocomplete.Select2ListView): - '''View to provide VIAF suggestions for autocomplete lookup. + """View to provide VIAF suggestions for autocomplete lookup. Based on :class:`dal.autocompleteSelect2ListView`. Expects search term as query string parameter `q`. Returns viaf URI as identifier and display form as text. - ''' + """ def get(self, request, *args, **kwargs): """Return JSON with suggested VIAF ids and display names.""" viaf = ViafAPI() # search for specific kind of name if set - nametype = self.kwargs.get('nametype', None) - if nametype == 'personal': + nametype = self.kwargs.get("nametype", None) + if nametype == "personal": result = viaf.find_person(self.q) else: result = viaf.search(self.q) # check for empty search result and return empty json response if result is None: - return JsonResponse({'results': []}) + return JsonResponse({"results": []}) - return JsonResponse({ - 'results': [dict( - # id=viaf.uri_from_id(item.recordData.viafID), - id=item.uri, - id_number=item.viaf_id, - text=item.label, - nametype=item.nametype, - # possibly useful to include, since we have them (for people) - birth=item.recordData.VIAFCluster.birthDate, - death=item.recordData.VIAFCluster.deathDate, - # exclude any names that are not personal - ) for item in result] - }) + return JsonResponse( + { + "results": [ + { + # 'id': viaf.uri_from_id(item.recordData.viafID), + "id": item.uri, + "id_number": item.viaf_id, + "text": item.label, + "nametype": item.nametype, + # possibly useful to include, since we have them (for people) + "birth": item.recordData.VIAFCluster.birthDate, + "death": item.recordData.VIAFCluster.deathDate, + # exclude any names that are not personal + } + for item in result + ] + } + ) diff --git a/src/viapy/widgets.py b/src/viapy/widgets.py index 885c6f3..b0bc808 100644 --- a/src/viapy/widgets.py +++ b/src/viapy/widgets.py @@ -11,8 +11,7 @@ def render(self, name, value, renderer=None, attrs=None): # so when a value is set, add it to the list of choices if value: self.choices = [(value, value)] - widget = super(ViafWidget, self).render(name, value, attrs) + widget = super().render(name, value, attrs) return mark_safe( - '%s


%s

' - % (widget, value or "", value or "") + f'{widget}


{value or ""}

' ) diff --git a/tests/test_api.py b/tests/test_api.py index 4cd5eba..c916df2 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -1,16 +1,16 @@ import json -import os +import pathlib from unittest.mock import Mock, patch -import requests import rdflib +import requests -from viapy.api import ViafAPI, ViafEntity, SRUResult, SRUItem +from viapy.api import SRUItem, SRUResult, ViafAPI, ViafEntity -FIXTURES_PATH = os.path.join(os.path.dirname(__file__), "fixtures") +FIXTURES_PATH = pathlib.Path(__file__).parent / "fixtures" -class TestViafAPI(object): +class TestViafAPI: def test_get_uri(self): assert ViafAPI.uri_from_id("1234") == "http://viaf.org/viaf/1234" # numeric id should also work @@ -51,8 +51,8 @@ def test_search(self, mockrequests): viaf = ViafAPI() mockrequests.codes = requests.codes - sru_fixture = os.path.join(FIXTURES_PATH, "sru_search.json") - with open(sru_fixture) as srufile: + sru_fixture = FIXTURES_PATH / "sru_search.json" + with sru_fixture.open() as srufile: mock_result = json.load(srufile) mockrequests.get.return_value.status_code = requests.codes.ok mockrequests.get.return_value.json.return_value = mock_result @@ -85,7 +85,7 @@ def test_find_person(self): with patch.object(viaf, "search") as mocksearch: viaf.find_person(term) - mocksearch.assert_called_with('local.personalNames all "%s"' % term) + mocksearch.assert_called_with(f'local.personalNames all "{term}"') def test_find_corporate(self): viaf = ViafAPI() @@ -93,7 +93,7 @@ def test_find_corporate(self): with patch.object(viaf, "search") as mocksearch: viaf.find_corporate(term) - mocksearch.assert_called_with('local.corporateNames all "%s"' % term) + mocksearch.assert_called_with(f'local.corporateNames all "{term}"') def test_find_place(self): viaf = ViafAPI() @@ -101,13 +101,13 @@ def test_find_place(self): with patch.object(viaf, "search") as mocksearch: viaf.find_place(term) - mocksearch.assert_called_with('local.geographicNames all "%s"' % term) + mocksearch.assert_called_with(f'local.geographicNames all "{term}"') -class TestViafEntity(object): +class TestViafEntity: test_id = 102333412 test_uri = "http://viaf.org/viaf/102333412" - rdf_fixture = os.path.join(FIXTURES_PATH, "102333412_rdf.xml") + rdf_fixture = FIXTURES_PATH / "102333412_rdf.xml" def test_init(self): # numeric id (either int or string should work) @@ -137,9 +137,13 @@ def test_rdf(self, mockrdflib, mockrequests): # should call requests.get on the uri, initialize a graph and parse data assert ent.rdf == mockrdflib.Graph.return_value mockrdflib.Graph.assert_called_with() - mockrequests.get.assert_called_with(self.test_uri, headers={"Accept": "application/rdf+xml"}) - mockrdflib.Graph.return_value.parse.assert_called_with(data=mock_response.text, format="xml") - mockrequests.raise_for_status.assert_called_once + mockrequests.get.assert_called_with( + self.test_uri, headers={"Accept": "application/rdf+xml"} + ) + mockrdflib.Graph.return_value.parse.assert_called_with( + data=mock_response.text, format="xml" + ) + mock_response.raise_for_status.assert_called_once() def test_properties(self): # use viaf id matching fixture rdf file @@ -165,8 +169,8 @@ def test_year_from_isodate(self): def test_sru_result(): # test SRUResult class properties - sru_fixture = os.path.join(FIXTURES_PATH, "sru_search.json") - with open(sru_fixture) as srufile: + sru_fixture = FIXTURES_PATH / "sru_search.json" + with sru_fixture.open() as srufile: sru_data = json.load(srufile) sru_res = SRUResult(sru_data) assert sru_res.total_results == 8 @@ -177,8 +181,8 @@ def test_sru_result(): def test_sru_item(): # test SRUItem class - sru_fixture = os.path.join(FIXTURES_PATH, "sru_search.json") - with open(sru_fixture) as srufile: + sru_fixture = FIXTURES_PATH / "sru_search.json" + with sru_fixture.open() as srufile: sru_data = json.load(srufile) sru_item = SRUResult(sru_data).records[0] assert sru_item.uri == "http://viaf.org/viaf/100260717/" diff --git a/tests/test_django.py b/tests/test_django.py index 95fde73..83206ec 100644 --- a/tests/test_django.py +++ b/tests/test_django.py @@ -2,8 +2,8 @@ import json from unittest.mock import patch -from attrdict import AttrDict import pytest +from attrdict import AttrDict try: import django @@ -19,120 +19,120 @@ from unittest import TestCase -@pytest.mark.skipif(django is None, reason='Requires Django') -class TestViafWidget(object): - +@pytest.mark.skipif(django is None, reason="Requires Django") +class TestViafWidget: def test_render(self): widget = ViafWidget() # no value set - should not error - rendered = widget.render('person', None, {'id': 123}) - assert '


' \ - in rendered + rendered = widget.render("person", None, {"id": 123}) + assert '


' in rendered # test marked as "safe"? # uri value set - should be included in generated link *and* # set as an option - uri = 'http://viaf.org/viaf/13103985/' - rendered = widget.render('person', uri, {'id': 1234}) - assert '%(uri)s' \ - % {'uri': uri} in rendered + uri = "http://viaf.org/viaf/13103985/" + rendered = widget.render("person", uri, {"id": 1234}) + assert f'{uri}' in rendered # value should be set as an option to preserve existing # value when the form is submitted - assert '