fix: merge identifiers across a book's formats
Identifiers are a collection, but the whole dict was replaced per format, so the last file to report won outright — an EPUB declaring an ASIN, a Google id and a Calibre id kept none of them once a PDF contributed one ISBN. Accumulate instead, letting a declared identifier outrank one scraped off a page.
This commit is contained in:
@@ -124,15 +124,36 @@ class Extractor:
|
|||||||
# EPUB tends to give better metadata results over pdf
|
# EPUB tends to give better metadata results over pdf
|
||||||
sorted_files = sorted(files, key=lambda f: Extractor._get_file_priority(f))
|
sorted_files = sorted(files, key=lambda f: Extractor._get_file_priority(f))
|
||||||
|
|
||||||
|
# Identifiers accumulate across formats instead of replacing each other. Every
|
||||||
|
# other field is a single value where the later, better-trusted format simply
|
||||||
|
# wins, but identifiers are a *collection*: a book holding an EPUB and a PDF
|
||||||
|
# genuinely carries what both of them declare, and merging the dict wholesale
|
||||||
|
# threw away everything the earlier format found. An EPUB that declares an
|
||||||
|
# ASIN, a Google volume id and a Calibre id kept none of them once a PDF
|
||||||
|
# contributed a single ISBN.
|
||||||
|
identifiers: dict[str, str] = {}
|
||||||
|
|
||||||
for file in sorted_files:
|
for file in sorted_files:
|
||||||
match get_file_extension(file):
|
match get_file_extension(file):
|
||||||
case "epub":
|
case "epub":
|
||||||
metadata = metadata | await EpubExtractor.extract_metadata(file)
|
extracted = await EpubExtractor.extract_metadata(file)
|
||||||
case "pdf":
|
case "pdf":
|
||||||
metadata = metadata | await PdfExtractor.extract_metadata(file)
|
extracted = await PdfExtractor.extract_metadata(file)
|
||||||
case _:
|
case _:
|
||||||
break
|
break
|
||||||
|
|
||||||
|
# First writer wins per name, and the files are already ordered by how
|
||||||
|
# far their metadata can be trusted. A `dc:identifier` the publisher
|
||||||
|
# declared outranks an ISBN scraped out of a PDF's copyright page, which
|
||||||
|
# routinely prints the ISBNs of other formats and older editions too.
|
||||||
|
for name, value in (extracted.pop("identifiers", None) or {}).items():
|
||||||
|
identifiers.setdefault(name, value)
|
||||||
|
|
||||||
|
metadata = metadata | extracted
|
||||||
|
|
||||||
|
if identifiers:
|
||||||
|
metadata["identifiers"] = identifiers
|
||||||
|
|
||||||
# Get metadata from file names
|
# Get metadata from file names
|
||||||
for file in files:
|
for file in files:
|
||||||
metadata = FilenameExtractor.extract_metadata(file) | metadata
|
metadata = FilenameExtractor.extract_metadata(file) | metadata
|
||||||
|
|||||||
@@ -1,7 +1,7 @@
|
|||||||
import pytest
|
import pytest
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from datetime import date
|
from datetime import date
|
||||||
from chitai.services.metadata_extractor import EpubExtractor
|
from chitai.services.metadata_extractor import EpubExtractor, Extractor, PdfExtractor
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio()
|
@pytest.mark.asyncio()
|
||||||
@@ -15,3 +15,62 @@ class TestEpubExtractor:
|
|||||||
assert metadata["authors"] == ["Herman Melville"]
|
assert metadata["authors"] == ["Herman Melville"]
|
||||||
assert metadata["published_date"] == date(year=2001, month=7, day=1)
|
assert metadata["published_date"] == date(year=2001, month=7, day=1)
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
EPUB = Path("tests/data_files/Metamorphosis - Franz Kafka.epub")
|
||||||
|
PDF = Path("tests/data_files/Calculus Made Easy - Silvanus Thompson.pdf")
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio()
|
||||||
|
class TestIdentifierMerging:
|
||||||
|
"""A book's formats each contribute identifiers; none of them replaces the rest."""
|
||||||
|
|
||||||
|
async def test_every_format_contributes(self, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||||
|
"""
|
||||||
|
Identifiers are a collection, not a single value.
|
||||||
|
|
||||||
|
Merging the whole dict meant the last format to report won outright: an EPUB
|
||||||
|
declaring an ASIN, a Google volume id and a Calibre id kept none of them once
|
||||||
|
a PDF contributed one ISBN.
|
||||||
|
"""
|
||||||
|
|
||||||
|
async def epub(_file):
|
||||||
|
return {
|
||||||
|
"title": "How Linux Works",
|
||||||
|
"identifiers": {"isbn-13": "9781718500419", "asin": "1718500408"},
|
||||||
|
}
|
||||||
|
|
||||||
|
async def pdf(_file):
|
||||||
|
return {"identifiers": {"isbn-13": "9781718500402", "isbn-10": "1593270356"}}
|
||||||
|
|
||||||
|
monkeypatch.setattr(EpubExtractor, "extract_metadata", epub)
|
||||||
|
monkeypatch.setattr(PdfExtractor, "extract_metadata", pdf)
|
||||||
|
|
||||||
|
metadata = await Extractor.extract_metadata(
|
||||||
|
[Path("How Linux Works.epub"), Path("How Linux Works.pdf")]
|
||||||
|
)
|
||||||
|
|
||||||
|
assert metadata["identifiers"] == {
|
||||||
|
# Declared by the publisher's toolchain, so it outranks the PDF's, which
|
||||||
|
# was scraped off a copyright page that also prints the print edition's.
|
||||||
|
"isbn-13": "9781718500419",
|
||||||
|
"asin": "1718500408",
|
||||||
|
"isbn-10": "1593270356",
|
||||||
|
}
|
||||||
|
|
||||||
|
async def test_a_second_format_that_finds_nothing_erases_nothing(self) -> None:
|
||||||
|
"""The PDF fixture carries no ISBN, so it must leave the EPUB's alone."""
|
||||||
|
metadata = await Extractor.extract_metadata([EPUB, PDF])
|
||||||
|
|
||||||
|
assert metadata["identifiers"] == {"id": "http://www.gutenberg.org/5200"}
|
||||||
|
|
||||||
|
async def test_one_format_on_its_own_is_unaffected(self) -> None:
|
||||||
|
metadata = await Extractor.extract_metadata([EPUB])
|
||||||
|
|
||||||
|
assert metadata["identifiers"] == {"id": "http://www.gutenberg.org/5200"}
|
||||||
|
|
||||||
|
async def test_no_identifiers_anywhere_leaves_the_field_absent(self) -> None:
|
||||||
|
"""An empty dict would count as extracted metadata and overwrite nothing."""
|
||||||
|
metadata = await Extractor.extract_metadata([PDF])
|
||||||
|
|
||||||
|
assert "identifiers" not in metadata
|
||||||
|
|||||||
Reference in New Issue
Block a user