Skip to content

Fix regressions from caching refactoring #958

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kba merged 2 commits into from
Nov 30, 2022
Merged

Fix regressions from caching refactoring #958

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
862ece1
OcrdMets.remove_file_group with caching: shallow copy DictValues
kba Nov 29, 2022
d0b5513
OcrdMets: fix (return) types when caching is on, #957
kba Nov 29, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions ocrd_models/ocrd_models/ocrd_mets.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ def remove_file_group(self, USE, recursive=False, force=False):
if files:
if not recursive:
raise Exception("fileGrp %s is not empty and recursive wasn't set" % USE)
for f in files:
for f in list(files):
self.remove_one_file(ID=f.get('ID'), fileGrp=f.getparent().get('USE'))

if self._cache_flag:
Expand Down Expand Up @@ -557,7 +557,7 @@ def physical_pages(self):
List all page IDs (the ``@ID`` of each physical ``mets:structMap`` ``mets:div``)
"""
if self._cache_flag:
return self._page_cache.values()
return list(self._page_cache.keys())

return self._tree.getroot().xpath(
'mets:structMap[@TYPE="PHYSICAL"]/mets:div[@TYPE="physSequence"]/mets:div[@TYPE="page"]/@ID',
Expand Down Expand Up @@ -685,7 +685,7 @@ def remove_physical_page(self, ID):
mets_div = self._tree.getroot().xpath(
'mets:structMap[@TYPE="PHYSICAL"]/mets:div[@TYPE="physSequence"]/mets:div[@TYPE="page"][@ID="%s"]' % ID,
namespaces=NS)
if mets_div is not None:
if mets_div:
mets_div[0].getparent().remove(mets_div[0])
if self._cache_flag:
del self._page_cache[ID]
Expand Down
23 changes: 12 additions & 11 deletions tests/model/test_ocrd_mets.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,15 @@ def _fixture(request):
yield mets


@pytest.fixture(name='sbb_directory_ocrd_mets', params=CACHING_ENABLED)
def _fixture_sbb(tmp_path, request):
src_path = assets.path_to('SBB0000F29300010000/data')
dst_path = tmp_path / 'SBB_directory'
shutil.copytree(src_path, dst_path)
mets_path = str(join(dst_path, 'mets.xml'))
yield OcrdMets(filename=mets_path, cache_flag=request.param)


def test_unique_identifier():
mets = OcrdMets(filename=assets.url_of('SBB0000F29300010000/data/mets.xml'))
assert mets.unique_identifier == 'http://resolver.staatsbibliothek-berlin.de/SBB0000F29300010000', 'Right identifier'
Expand Down Expand Up @@ -89,7 +98,8 @@ def test_find_all_files_local_only(sbb_sample_01):

def test_physical_pages(sbb_sample_01):
assert len(sbb_sample_01.physical_pages) == 3, '3 physical pages'

assert isinstance(sbb_sample_01.physical_pages, list)
assert isinstance(sbb_sample_01.physical_pages[0], str)

def test_physical_pages_from_empty_mets():
mets = OcrdMets(content="<mets></mets>")
Expand All @@ -98,15 +108,6 @@ def test_physical_pages_from_empty_mets():
assert len(mets.physical_pages) == 1, '1 physical page'


@pytest.fixture(name='sbb_directory_ocrd_mets')
def _fixture_sbb(tmp_path):
src_path = assets.path_to('SBB0000F29300010000/data')
dst_path = tmp_path / 'SBB_directory'
shutil.copytree(src_path, dst_path)
mets_path = str(join(dst_path, 'mets.xml'))
yield OcrdMets(filename=mets_path)


def test_physical_pages_for_fileids(sbb_directory_ocrd_mets):
assert sbb_directory_ocrd_mets.get_physical_pages(
for_fileIds=['FILE_0002_IMAGE']) == ['PHYS_0002']
Expand Down Expand Up @@ -265,6 +266,7 @@ def test_remove_page(sbb_directory_ocrd_mets):
def test_remove_physical_page_fptr(sbb_directory_ocrd_mets):
assert sbb_directory_ocrd_mets.get_physical_pages(for_fileIds=['FILE_0002_IMAGE']), ['PHYS_0002']
sbb_directory_ocrd_mets.remove_physical_page_fptr('FILE_0002_IMAGE')
sbb_directory_ocrd_mets.remove_physical_page_fptr('FILE_0002_IMAGE')
assert sbb_directory_ocrd_mets.get_physical_pages(for_fileIds=['FILE_0002_IMAGE']), [None]


Expand Down Expand Up @@ -346,7 +348,6 @@ def test_merge(sbb_sample_01):
sbb_sample_01.merge(other_mets, fileGrp_mapping={'OCR-D-IMG': 'FOO'})
assert len(sbb_sample_01.file_groups) == 18


def test_invalid_filegrp():
"""addresses https://github.com/OCR-D/core/issues/746"""

Expand Down