Replace STCN reader

edpop_explorer/cerl.py

@@ -15,6 +15,7 @@ class CERLReader(Reader):
     This is an abstract class -- to use, derive from this class, set the
     ``API_URL``, ``API_BY_ID_BASE_URL`` and ``LINK_BASE_URL`` constant
     attributes, and implement the ``_convert_record`` class method."""
+
     API_URL: str
     """The base URL of the search API, of the form ``https://data.cerl.org/<CATALOGUE>/_search``."""
     API_BY_ID_BASE_URL: str
@@ -49,7 +50,6 @@ def _perform_query(self, start_record: int, maximum_records: Optional[int]) -> L
         assert isinstance(self.prepared_query, str)
         if maximum_records is None:
             maximum_records = self.DEFAULT_RECORDS_PER_PAGE
-        print(f'The query is: {self.prepared_query}')
         try:
             response = requests.get(
                 self.API_URL,
@@ -64,9 +64,7 @@ def _perform_query(self, start_record: int, maximum_records: Optional[int]) -> L
                     'Accept': 'application/json'
                 }
             ).json()
-        except (
-            requests.exceptions.RequestException
-        ) as err:
+        except requests.exceptions.RequestException as err:
             raise ReaderError('Error during server request: ' + str(err))
 
         # TODO: check for error responses
@@ -78,16 +76,7 @@ def _perform_query(self, start_record: int, maximum_records: Optional[int]) -> L
         except KeyError:
             raise ReaderError('Number of hits not given in server response')
 
-        if 'rows' not in response:
-            # There are no rows in the response, so stop here
-            return []
-
-        records: List[Record] = []
-        for rawrecord in response['rows']:
-            record = self._convert_record(rawrecord)
-            records.append(record)
-
-        return records
+        return [self._convert_record(x) for x in response['rows']] if 'rows' in response else []
 
     @classmethod
     def transform_query(cls, query) -> str:

edpop_explorer/edpopxshell.py

@@ -183,7 +183,7 @@ def do_stcn(self, args) -> None:
         self._query(STCNReader, args)
 
     def do_stcnpers(self, args) -> None:
-        'Short Title Catalogue Netherlands – Persons'
+        'Short Title Catalogue Netherlands – Persons (authors and other contributors)'
         self._query(STCNPersonsReader, args)
 
     def do_sbti(self, args) -> None:

edpop_explorer/readers/stcn.py

@@ -13,6 +13,11 @@ def _remove_markup(input_str: str) -> str:
 
 
 def safeget(dictionary: Optional[dict], attribute_chain: tuple, first: bool = False):
+    """Safely get a (nested) attribute in a JSON-like structure. If the
+    result is a list and ``first`` is ``True``, return the first item
+    of the list."""
+    if len(attribute_chain) == 0:
+        raise ValueError("The attribute_chain argument cannot be empty")
     attribute = attribute_chain[0]
     if dictionary is None or attribute not in dictionary:
         return None
@@ -25,6 +30,20 @@ def safeget(dictionary: Optional[dict], attribute_chain: tuple, first: bool = Fa
         return safeget(value, attribute_chain[1:], first)
 
 
+def _wrap_contributor(actor_data: dict) -> ContributorField:
+    field = ContributorField(actor_data['preferred'])
+    field.name = actor_data['preferred']
+    field.role = safeget(actor_data, ('role',), first=True)
+    return field
+
+
+def _wrap_holding(holding_data: dict) -> Field:
+    institution = safeget(holding_data, ("data", "institutionName"))
+    shelfmark = safeget(holding_data, ("data", "shelfmark"))
+    summary = f"{institution} - {shelfmark}"
+    return Field(summary)
+
+
 class STCNBaseReader(CERLReader):
     """STCN uses the same search API for its bibliographical records and
     its biographical records (persons and publishers/printers), but the
@@ -34,6 +53,8 @@ class STCNBaseReader(CERLReader):
 
 
 class STCNPersonsReader(STCNBaseReader):
+    """STCN Persons reader. This reader does not include printers and
+    publishers, because they are in a separate database."""
     API_BY_ID_BASE_URL = 'https://data.cerl.org/stcn_persons/'
     LINK_BASE_URL = 'https://data.cerl.org/stcn_persons/'
     CATALOG_URIREF = URIRef(
@@ -114,24 +135,15 @@ def _get_contributors(cls, rawrecord: dict) -> List[Field]:
         actors = safeget(rawrecord, ("data", "agent"))
         if not actors:
             return []
-        contributors = []
-        for actor in actors:
-            name = actor.get("preferred", None)
-            if name is None:
-                continue
-            contributor = ContributorField(name)
-            contributor.name = name
-            contributor.role = safeget(actor, ('role',), first=True)
-            contributors.append(contributor)
-        return contributors
+        return [_wrap_contributor(x) for x in actors if x.get('preferred')]
 
     @classmethod
     def _get_publisher_or_printer(cls, rawrecord: dict) -> Optional[Field]:
         # TODO: support multiple publishers/printers
         provision_agent = safeget(rawrecord, ("data", "provisionAgent"), first=True)
         if provision_agent is None:
             return None
-        name = provision_agent.get("preferred", None)
+        name = safeget(provision_agent, ("preferred",))
         if name is None:
             return None
         field = Field(name)
@@ -211,13 +223,7 @@ def _get_holdings(cls, rawrecord: dict) -> List[Field]:
         holdings = safeget(rawrecord, ("data", "holdings"))
         if holdings is None:
             return []
-        fields = []
-        for holding in holdings:
-            institution = safeget(holding, ("data", "institutionName"))
-            shelfmark = safeget(holding, ("data", "shelfmark"))
-            summary = f"{institution} - {shelfmark}"
-            fields.append(Field(summary))
-        return fields
+        return [_wrap_holding(x) for x in holdings]
 
     @classmethod
     def _convert_record(cls, rawrecord: dict) -> BibliographicalRecord:

tests/readers/test_stcn.py

@@ -0,0 +1,46 @@
+import pytest
+
+from edpop_explorer.readers.stcn import safeget
+
+
+def test_safeget_empty_attribute_chain():
+    with pytest.raises(ValueError):
+        safeget(None, ())
+
+def test_safeget_empty_dict():
+    assert safeget({}, ("attribute",)) is None
+
+def test_safeget_none():
+    assert safeget(None, ("attribute",)) is None
+
+def test_safeget_simple():
+    assert safeget({"attribute": "value"}, ("attribute",)) == "value"
+
+def test_safeget_nested():
+    assert safeget(
+        {
+            "attribute": {"attribute2": "value"}
+        }, ("attribute", "attribute2")
+    ) == "value"
+
+def test_safeget_nested_first_attribute_none():
+    assert safeget({
+        "attribute": None
+    }, ("attribute", "attribute2")) is None
+
+def test_safeget_nested_first_attribute_nonexistent():
+    assert safeget({
+        "other_attribute": None
+    }, ("attribute", "attribute2")) is None
+
+def test_safeget_nested_second_attribute_nonexistent():
+    assert safeget({
+        "attribute": {
+            "other_attribute": "value"
+        }
+    }, ("attribute", "attribute2")) is None
+
+def test_safeget_first():
+    assert safeget({
+        "attribute": ["value1", "value2"]
+    }, ("attribute",), True) == "value1"