From e340430f30832bde629e8ec420269ac18dc02e9c Mon Sep 17 00:00:00 2001 From: caufieldjh Date: Thu, 9 Jul 2026 15:05:08 -0400 Subject: [PATCH] perf(pmid): derive abstract from the single article-XML fetch Addresses the deferred note 2 from PR #57 review. Previously fetch() made two efetch calls per PMID: one for the abstract (rettype=abstract, text) and one for the article XML (MeSH terms + publication types). The article XML already contains the abstract, so parse it from there and drop the second call along with its rate-limit sleep. - New _parse_abstract reconstructs prose from Abstract/AbstractText, preserving structured-section labels (e.g. "METHODS:"); replaces the now-removed _fetch_abstract. - content is now the abstract prose only, without the citation-header boilerplate the text fetch used to prepend. Supporting-text validation matches claims against abstract/full-text prose, not headers, so this is neutral-to-cleaner; a record with no abstract now correctly yields no content instead of a header-only stub. Tests: structured/unstructured/absent/empty abstract parsing, plus a fetch() test asserting exactly one efetch call backs all three fields. Co-Authored-By: Claude Opus 4.8 --- .../etl/sources/pmid.py | 52 +++++++++----- tests/test_sources.py | 69 ++++++++++++++++--- 2 files changed, 94 insertions(+), 27 deletions(-) diff --git a/src/linkml_reference_validator/etl/sources/pmid.py b/src/linkml_reference_validator/etl/sources/pmid.py index 0a7fd5a..097b376 100644 --- a/src/linkml_reference_validator/etl/sources/pmid.py +++ b/src/linkml_reference_validator/etl/sources/pmid.py @@ -137,8 +137,10 @@ def fetch( year = str(pub_date)[:4] if pub_date else "" doi = str(record_dict.get("DOI", "")) if record_dict.get("DOI") else "" - abstract = self._fetch_abstract(pmid, config) + # A single efetch of the article XML backs the abstract, MeSH terms, + # and publication types, so we don't round-trip to NCBI three times. article_xml = self._fetch_pubmed_xml(pmid, config) + abstract = self._parse_abstract(article_xml) if article_xml else None keywords = self._parse_mesh_terms(article_xml) if article_xml else None publication_types = ( self._parse_publication_types(article_xml) if article_xml else None @@ -188,29 +190,47 @@ def _parse_authors(self, author_list: list) -> list[str]: """ return [str(author) for author in author_list if author] - def _fetch_abstract( - self, pmid: str, config: ReferenceValidationConfig - ) -> Optional[str]: - """Fetch abstract for a PMID. + def _parse_abstract(self, soup: BeautifulSoup) -> Optional[str]: + """Parse the abstract from a PubMed article XML document. + + Reconstructs the abstract prose from ``Abstract/AbstractText`` nodes. + Structured abstracts label each section (e.g. ``Label="METHODS"``); + the label is preserved as a ``"METHODS:"`` prefix and sections are + joined by blank lines, mirroring how PubMed renders them as text. Args: - pmid: PubMed ID - config: Configuration for rate limiting + soup: Parsed PubMed article XML Returns: - Abstract text if available + Abstract text if available, otherwise None + + Examples: + >>> from bs4 import BeautifulSoup + >>> xml = ''' + ... We ran a trial. + ... It worked. + ... ''' + >>> PMIDSource()._parse_abstract(BeautifulSoup(xml, "xml")) + 'METHODS: We ran a trial.\\n\\nRESULTS: It worked.' + >>> xml = 'A summary.' + >>> PMIDSource()._parse_abstract(BeautifulSoup(xml, "xml")) + 'A summary.' """ - time.sleep(config.rate_limit_delay) + abstract = soup.find("Abstract") - handle = Entrez.efetch(db="pubmed", id=pmid, - rettype="abstract", retmode="text") - abstract_text = handle.read() - handle.close() + if not abstract: + return None - if abstract_text and len(abstract_text) > 50: - return str(abstract_text) + sections = [] + for node in abstract.find_all("AbstractText"): + text = node.get_text().strip() + if not text: + continue + label = node.get("Label") + sections.append(f"{label}: {text}" if label else text) - return None + joined = "\n\n".join(sections) + return joined if joined else None def _fetch_pubmed_xml( self, pmid: str, config: ReferenceValidationConfig diff --git a/tests/test_sources.py b/tests/test_sources.py index 0fc6a53..2a50d27 100644 --- a/tests/test_sources.py +++ b/tests/test_sources.py @@ -326,6 +326,10 @@ def test_get_pmcid_handles_entrez_error(
An illustrative case + + We describe the case. + The patient recovered. + Journal Article Case Reports @@ -383,13 +387,56 @@ def test_parse_mesh_terms_empty_list(self, source): assert source._parse_mesh_terms(soup) is None + # --- Abstract parsing (issue #56 note 2: fold into single XML fetch) --- + + def test_parse_abstract_structured(self, source): + """Structured abstracts should keep their section labels.""" + from bs4 import BeautifulSoup + + soup = BeautifulSoup(self._ARTICLE_XML, "xml") + + assert source._parse_abstract(soup) == ( + "METHODS: We describe the case.\n\nRESULTS: The patient recovered." + ) + + def test_parse_abstract_unstructured(self, source): + """A single unlabelled AbstractText should return its prose verbatim.""" + from bs4 import BeautifulSoup + + soup = BeautifulSoup( + "Just one paragraph.", + "xml", + ) + + assert source._parse_abstract(soup) == "Just one paragraph." + + def test_parse_abstract_absent(self, source): + """Should return None when there is no Abstract element.""" + from bs4 import BeautifulSoup + + soup = BeautifulSoup( + "", "xml" + ) + + assert source._parse_abstract(soup) is None + + def test_parse_abstract_empty(self, source): + """An Abstract with only empty AbstractText should yield None.""" + from bs4 import BeautifulSoup + + soup = BeautifulSoup( + "", "xml" + ) + + assert source._parse_abstract(soup) is None + @patch("linkml_reference_validator.etl.sources.pmid.Entrez.efetch") @patch("linkml_reference_validator.etl.sources.pmid.Entrez.esummary") @patch("linkml_reference_validator.etl.sources.pmid.Entrez.read") - def test_fetch_populates_publication_types( + def test_fetch_uses_single_efetch( self, mock_read, mock_esummary, mock_efetch, source, config ): - """fetch() should surface publication types on the ReferenceContent.""" + """fetch() should derive abstract, MeSH, and pub types from one efetch.""" mock_read.return_value = [ { "Title": "An illustrative case", @@ -400,19 +447,19 @@ def test_fetch_populates_publication_types( } ] - def _efetch(*args, **kwargs): - handle = MagicMock() - if kwargs.get("rettype") == "abstract": - handle.read.return_value = "x" * 100 - else: - handle.read.return_value = self._ARTICLE_XML - return handle - - mock_efetch.side_effect = _efetch + handle = MagicMock() + handle.read.return_value = self._ARTICLE_XML + mock_efetch.return_value = handle ref = source.fetch("12345678", config) + # A single article-XML fetch backs all three fields — no second call. + assert mock_efetch.call_count == 1 assert ref is not None + assert ref.content == ( + "METHODS: We describe the case.\n\nRESULTS: The patient recovered." + ) + assert ref.content_type == "abstract_only" assert ref.publication_types == [ "Journal Article", "Case Reports",