Redesign SourceItem.extract() return contract

- extract() now always returns SourceItem (never None)
- Add new_ident field: set on redirect, replaces patch[0] convention
- parse() is now void and receives ident as parameter
- Connectors set self.prior_ident directly when primary-property lookup
  confirms absence (ADS 0-doc bibcode, EuropePMC PMID, ArxivItem P818,
  ORCID missing path)
- item.py transform() uses new_ident instead of patch[0] for replacement

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Author: ghuron

src/wdpy/connectors/ads.py

@@ -15,14 +15,14 @@ def make_request(cls, ident: Statement) -> Optional[Request]:
                 headers={'Authorization': 'Bearer ogOoi0uDxIebyeseB3tAbf5mBTJxXQQWQqE5TW40'}
             )
 
-    def parse(self, text: str) -> bool:
+    def parse(self, text: str, ident: Statement) -> None:
         docs = json.loads(text).get('response', {}).get('docs', [])
         if len(docs) != 1:
-            if self.patch and self.patch[0].mainsnak.property == 'P819':
-                logging.warning('Got %d results for %s', len(docs), self.patch[0].mainsnak.value)
-                self.patch = []
-                return True
-            return False
+            if ident.mainsnak.property == 'P819':
+                logging.warning('Got %d results for %s', len(docs), ident.mainsnak.value)
+                if len(docs) == 0:
+                    self.prior_ident = ident
+            return
         d = docs[0]
         if 'page' in d:
             p = d['page'][0] if isinstance(d['page'], list) else d['page']
@@ -36,13 +36,12 @@ def parse(self, text: str) -> bool:
         self.obtain(d, self._config.get('fields', {}), self._config.get('translate', {}))
         for i, name in enumerate(d.get('author', []), 1):
             self.add_author(name, _get_orcid(d, i - 1))
-        for ident in d.get('identifier', []):
-            if ident.startswith('arXiv:'):
-                self.add_claim('P818', ident[6:])
-            elif ident.startswith('10.') and 'ARXIV' not in ident.upper():
-                self.add_claim('P356', ident)
+        for identifier in d.get('identifier', []):
+            if identifier.startswith('arXiv:'):
+                self.add_claim('P818', identifier[6:])
+            elif identifier.startswith('10.') and 'ARXIV' not in identifier.upper():
+                self.add_claim('P356', identifier)
         self.add_claim('P31', 'Q13442814')
-        return True
 
 
 def _get_orcid(d: dict, idx: int) -> Optional[str]:

src/wdpy/connectors/arxiv.py

@@ -1,15 +1,17 @@
 from xml.etree import ElementTree
-from wdpy import SourceItem
+from wdpy import SourceItem, Statement
 
 class ArxivItem(SourceItem):
-    def parse(self, text: str) -> bool:
+    def parse(self, text: str, ident: Statement) -> None:
         ns = self._config['namespaces']
         if (entry := ElementTree.fromstring(text).find('w3:entry', ns)) is None:
-            self.patch = []
-            return True
+            if ident.mainsnak.property == 'P818':
+                self.prior_ident = ident
+            return
         id_text = getattr(entry.find('w3:id', ns), 'text', '') or ''
-        if self.patch and self.patch[0].mainsnak.property == 'P818' and '/abs/' not in id_text:
-            return False
+        if ident.mainsnak.property == 'P818' and '/abs/' not in id_text:
+            self.prior_ident = ident
+            return
         arxiv_id = id_text.split('/')[-1].split('v')[0]
         self.add_claim('P31', 'Q13442814')
         if arxiv_id: self.add_claim('P953', 'https://arxiv.org/pdf/' + arxiv_id)
@@ -22,8 +24,3 @@ def parse(self, text: str) -> bool:
                 self.add_claim('P2093', name).set_qualifier('P1545', str(i := i + 1))
         if len(doi := entry.findall('arxiv:doi', ns)) == 1 and doi[0].text:
             self.add_claim('P356', doi[0].text.upper())
-        return True
-
-# r = ArxivItem.extract(Statement(Snak('P818', ('1309.0951',))))
-# r = ArxivItem.extract(Statement(Snak('P356', ('10.4171/161',))))
-# pass

src/wdpy/connectors/crossref.py

@@ -1,8 +1,7 @@
 import json
-from wdpy import SourceItem
+from wdpy import SourceItem, Statement
 
 
 class Crossref(SourceItem):
-    def parse(self, text: str) -> bool:
+    def parse(self, text: str, ident: Statement) -> None:
         self.obtain(json.loads(text), self._config.get('fields', {}), self._config.get('translate', {}))
-        return True

src/wdpy/connectors/europepmc.py

@@ -1,15 +1,14 @@
 import json
-from wdpy import SourceItem
+from wdpy import SourceItem, Statement
 
 
 class EuropePMC(SourceItem):
-    def parse(self, text: str) -> bool:
+    def parse(self, text: str, ident: Statement) -> None:
         results = json.loads(text).get('resultList', {}).get('result', [])
         if len(results) != 1:
-            if self.patch and self.patch[0].mainsnak.property == 'P698':
-                return False
-            self.patch = []
-            return True
+            if ident.mainsnak.property == 'P698':
+                self.prior_ident = ident
+            return
         d = results[0]
         fields = self._config.get('fields', {})
         translate = self._config.get('translate', {})
@@ -19,4 +18,3 @@ def parse(self, text: str) -> bool:
         for a in d.get('authorList', {}).get('author', []):
             orcid = a['authorId']['value'] if a.get('authorId', {}).get('type') == 'ORCID' else None
             self.add_author(a.get('firstName', '') + ' ' + a.get('lastName', ''), orcid)
-        return True

src/wdpy/connectors/orcid.py

@@ -11,16 +11,15 @@ def make_request(cls, ident: Statement) -> Optional[Request]:
             req.add_header('Accept', 'application/json')
         return req
 
-    def parse(self, text: str) -> bool:
+    def parse(self, text: str, ident: Statement) -> None:
         data = json.loads(text)
         if 'path' not in data:
-            self.patch = []
-            return True
+            self.prior_ident = ident
+            return
         self.add_claim('P496', data['path'].split('/')[1])
         self.add_claim('P31', 'Q5')
         self.add_claim('P106', 'Q1650915')
         for i in data.get('external-identifiers', {}).get('external-identifier', []):
             prop = self._config.get('fields', {}).get(i.get('external-id-type'))
             if prop:
                 self.add_claim(prop, i.get('external-id-value'))
-        return True

src/wdpy/item.py

@@ -61,12 +61,9 @@ def _extract_new(self, transformed: List[SourceItem]) -> Optional[SourceItem]:
                 if stmt.rank == 'deprecated' or not stmt.mainsnak.value:
                     continue
                 for model_type in SourceItem.get_extractors():
-                    if (model := model_type.extract(stmt)) is None:
+                    model = model_type.extract(stmt)
+                    if not (model.patch or model.prior_ident):
                         continue
-                    if model.patch:
-                        s = model.patch[0]
-                        if s.mainsnak.value and (s.mainsnak.property, s.mainsnak.value[0]) in deprecated:
-                            continue
                     if model not in transformed:
                         return model
         return None
@@ -112,11 +109,24 @@ def _ensure_loaded(self) -> None:
         self._loaded = True
 
     def transform(self, model: SourceItem) -> None:
-        if not isinstance(model, SourceItem) or model.patch is None:
+        if not isinstance(model, SourceItem):
+            return
+        if model.patch is None and not model.prior_ident and not model.new_ident:
             return
         self._ensure_loaded()
         source = model.get_db_ref()
 
+        if model.prior_ident:
+            prop = model.prior_ident.mainsnak.property
+            old_val = model.prior_ident.mainsnak.value
+            for stmt in self.claims.get(prop) or []:
+                if stmt.mainsnak.value == old_val:
+                    if model.new_ident:
+                        stmt.mainsnak.value = model.new_ident.mainsnak.value
+                    else:
+                        stmt.set_rank('deprecated', 'Q21441764')
+                    break
+
         if not model.patch:
             for prop in model.get_properties():
                 for stmt in self.claims.get(prop) or []:

src/wdpy/source_item.py

@@ -33,6 +33,8 @@ def request(url: str,
 class SourceItem:
     patch: Optional[List[Statement]] = None
     proposed_label: Optional[str] = None
+    prior_ident: Optional[Statement] = None
+    new_ident: Optional[Statement] = None
     _lookup_cache: ClassVar[Dict[str, Dict[Any, Optional[str]]]] = {}
     _registry: ClassVar[List[type]] = []
     _connectors_loaded: ClassVar[bool] = False
@@ -145,37 +147,50 @@ def register_new_item(statements: List[Statement]) -> None:
                               property_id, value, qid, SourceItem._lookup_cache[property_id][value])
             SourceItem._lookup_cache[property_id][value] = qid
 
-    def parse(self, text: str) -> bool:
+    def parse(self, text: str, ident: Statement) -> None:
         raise NotImplementedError('Subclasses must implement parse')
 
     @classmethod
-    def extract(cls, ident: Statement) -> Optional[SourceItem]:
-        """Returns None on retrieval failure; instance with empty patch if datasource
-        confirms the record does not exist; instance with non-empty patch where
-        patch[0] is the requested id (possibly updated from a redirect)."""
+    def extract(cls, ident: Statement) -> SourceItem:
+        """Fetch and parse the record for `ident`. Always returns a SourceItem.
+
+        Empty (patch/prior_ident/new_ident all None): no info — network error,
+        unrecognised property, or parse failure.
+
+        prior_ident set, new_ident None: ident confirmed gone; caller should
+        deprecate that statement.
+
+        prior_ident and new_ident set: ident redirected; caller should replace
+        old value with new_ident. patch carries any additional claims.
+
+        Only patch set: ident unchanged; patch carries new claims to merge.
+        """
         if not (req := cls.make_request(ident)):
-            return None
+            return cls()
         url = req.full_url
         try:
             resp = build_opener().open(req, timeout=30)
         except urllib.error.HTTPError as e:
             resp = e
         except Exception as e:
             logging.error('Request failed for %s: %s', url, e)
-            return None
+            return cls()
         handled = cls._config.get('extract', [])
         with resp:
             if ((code := resp.getcode()) == 404) and (404 in handled):
-                return cls(patch=[])
+                first_prop = next(iter(cls._config.get('properties', {})), None)
+                if ident.mainsnak.property == first_prop:
+                    return cls(prior_ident=ident)
+                return cls()
             destination = resp.geturl()
             text = resp.read().decode('utf-8')
             if code != 200:
                 logging.error(f'Returned code {code} for {url}, message: {text}')
         destination_url = destination or url
         redirected = 301 in handled and destination_url.lower() != url.lower()
+        item = cls()
         if redirected:
-            patch_ident = cls.update_ident(ident, destination_url)
-        else:
-            patch_ident = ident
-        item = cls(patch=[patch_ident])
-        return item if item.parse(text) else None
+            item.prior_ident = ident
+            item.new_ident = cls.update_ident(ident, destination_url)
+        item.parse(text, ident)
+        return item

tests/integration/test_ads.py

@@ -6,22 +6,30 @@
 class TestExtract(TestCase):
     def test_doi_returns_result(self):
         result = ADS.extract(Statement(Snak('P356', ('10.1088/2041-8205/763/1/L1',))))
-        self.assertEqual(result.patch[0].mainsnak.property, 'P356')
+        self.assertTrue(result.patch)
 
     def test_bibcode_returns_result(self):
         result = ADS.extract(Statement(Snak('P819', ('2013ApJ...763L...1M',))))
-        self.assertEqual(result.patch[0].mainsnak.property, 'P819')
+        self.assertTrue(result.patch)
 
     def test_arxiv_returns_result(self):
         result = ADS.extract(Statement(Snak('P818', ('1212.1162',))))
-        self.assertEqual(result.patch[0].mainsnak.property, 'P818')
+        self.assertTrue(result.patch)
 
-    def test_nonexistent_bibcode_returns_empty_patch(self):
+    def test_nonexistent_bibcode_sets_prior_ident(self):
         result = ADS.extract(Statement(Snak('P819', ('X',))))
-        self.assertEqual(result.patch, [])
+        self.assertIsNotNone(result.prior_ident)
+        self.assertIsNone(result.patch)
+        self.assertIsNone(result.new_ident)
 
-    def test_nonexistent_doi_returns_none(self):
-        self.assertIsNone(ADS.extract(Statement(Snak('P356', ('X',)))))
+    def test_nonexistent_doi_returns_empty(self):
+        result = ADS.extract(Statement(Snak('P356', ('X',))))
+        self.assertIsNone(result.prior_ident)
+        self.assertIsNone(result.patch)
+        self.assertIsNone(result.new_ident)
 
-    def test_nonexistent_arxiv_returns_none(self):
-        self.assertIsNone(ADS.extract(Statement(Snak('P818', ('X',)))))
+    def test_nonexistent_arxiv_returns_empty(self):
+        result = ADS.extract(Statement(Snak('P818', ('X',))))
+        self.assertIsNone(result.prior_ident)
+        self.assertIsNone(result.patch)
+        self.assertIsNone(result.new_ident)

tests/integration/test_arxiv.py

@@ -7,15 +7,20 @@
 class TestExtract(TestCase):
     def test_arxiv_id_returns_result(self):
         result = ArxivItem.extract(Statement(Snak('P818', ('1309.0951',))))
-        self.assertEqual(result.patch[0].mainsnak.property, 'P818')
+        self.assertTrue(result.patch)
 
     def test_doi_returns_result(self):
         result = ArxivItem.extract(Statement(Snak('P356', ('10.4171/161',))))
-        self.assertEqual(result.patch[0].mainsnak.property, 'P356')
+        self.assertTrue(result.patch)
 
-    def test_nonexistent_arxiv_id_returns_none(self):
-        self.assertIsNone(ArxivItem.extract(Statement(Snak('P818', ('X',)))))
+    def test_nonexistent_arxiv_id_sets_prior_ident(self):
+        result = ArxivItem.extract(Statement(Snak('P818', ('X',))))
+        self.assertIsNotNone(result.prior_ident)
+        self.assertIsNone(result.patch)
+        self.assertIsNone(result.new_ident)
 
-    def test_nonexistent_doi_returns_empty_patch(self):
+    def test_nonexistent_doi_returns_empty(self):
         result = ArxivItem.extract(Statement(Snak('P356', ('10.99999/nonexistent',))))
-        self.assertEqual(result.patch, [])
+        self.assertIsNone(result.prior_ident)
+        self.assertIsNone(result.patch)
+        self.assertIsNone(result.new_ident)

tests/integration/test_crossref.py

@@ -6,8 +6,10 @@
 class TestExtract(TestCase):
     def test_doi_returns_result(self):
         result = Crossref.extract(Statement(Snak('P356', ('10.1088/2041-8205/763/1/L1',))))
-        self.assertEqual(result.patch[0].mainsnak.property, 'P356')
+        self.assertTrue(result.patch)
 
-    def test_nonexistent_doi_returns_empty_patch(self):
+    def test_nonexistent_doi_sets_prior_ident(self):
         result = Crossref.extract(Statement(Snak('P356', ('X',))))
-        self.assertEqual(result.patch, [])
+        self.assertIsNotNone(result.prior_ident)
+        self.assertIsNone(result.patch)
+        self.assertIsNone(result.new_ident)