Skip to content

Commit

Permalink
feat(merge): Add convert_exceptions_to_warnings argument
Browse files Browse the repository at this point in the history
  • Loading branch information
jpmckinney committed May 8, 2024
1 parent ad1a254 commit 9b9c59c
Show file tree
Hide file tree
Showing 7 changed files with 161 additions and 38 deletions.
8 changes: 8 additions & 0 deletions docs/changelog.rst
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
Changelog
=========

1.1.13 (2024-05-08)
-------------------

Added
~~~~~

- :meth:`ocdskit.combine.merge` accepts a ``convert_exceptions_to_warnings`` argument.

1.1.12 (2024-05-07)
-------------------

Expand Down
2 changes: 1 addition & 1 deletion docs/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
author = "Open Contracting Partnership"

# The short X.Y version
version = "1.1.12"
version = "1.1.13"
# The full version, including alpha/beta/rc tags
release = version

Expand Down
21 changes: 16 additions & 5 deletions ocdskit/combine.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ def merge(
streaming: bool = False,
force_version: bool = False,
ignore_version: bool = False,
convert_exceptions_to_warnings: bool = False,
):
"""
Merges release packages and individual releases.
Expand All @@ -172,12 +173,13 @@ def merge(
if the calling code exhausts the generator before ``merge`` returns)
:param force_version: version to use instead of the version of the first release package or individual release
:param ignore_version: do not raise an error if the versions are inconsistent across items to merge
:param convert_exceptions_to_warnings: whether to convert inconsistent type errors from OCDS Merge to warnings
:raises InconsistentVersionError: if the versions are inconsistent across items to merge
:raises MissingOcidKeyError: if the release is missing an ``ocid`` field
:raises UnknownVersionError: if the OCDS version is not recognized
"""
with Packager(force_version=force_version, ignore_version=ignore_version) as packager:
packager.add(data)
with Packager(force_version=force_version) as packager:
packager.add(data, ignore_version=ignore_version)

if not schema and packager.version:
tag = get_ocds_patch_tag(packager.version)
Expand All @@ -197,10 +199,19 @@ def merge(
if publisher:
packager.package['publisher'] = publisher

yield from packager.output_package(merger, return_versioned_release=return_versioned_release,
use_linked_releases=use_linked_releases, streaming=streaming)
yield from packager.output_package(
merger,
return_versioned_release=return_versioned_release,
use_linked_releases=use_linked_releases,
streaming=streaming,
convert_exceptions_to_warnings=convert_exceptions_to_warnings,
)
else:
yield from packager.output_releases(merger, return_versioned_release=return_versioned_release)
yield from packager.output_releases(
merger,
return_versioned_release=return_versioned_release,
convert_exceptions_to_warnings=convert_exceptions_to_warnings,
)


def compile_release_packages(*args, **kwargs):
Expand Down
7 changes: 7 additions & 0 deletions ocdskit/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,3 +47,10 @@ class MissingReleasesWarning(OCDSKitWarning):

def __str__(self):
return 'item {0} has no "releases" field (check that it is a release package)'.format(*self.args)


class MergeErrorWarning(OCDSKitWarning):
"""Used when downgrading an OCDS Merge exception to a warning"""

def __str__(self):
return '{0}'.format(*self.args)
102 changes: 73 additions & 29 deletions ocdskit/packager.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@
from tempfile import NamedTemporaryFile
from typing import Union

from ocdskit.exceptions import InconsistentVersionError, MissingOcidKeyError
import ocdsmerge
from ocdsmerge.exceptions import InconsistentTypeError

from ocdskit.exceptions import InconsistentVersionError, MergeErrorWarning, MissingOcidKeyError
from ocdskit.util import (
_empty_record_package,
_remove_empty_optional_metadata,
Expand Down Expand Up @@ -40,6 +43,7 @@ def convert_json(string):
def _showwarning(showwarning, ocid):
def function(message, category, filename, lineno, file=None, line=None):
showwarning(f"{ocid}: {message}", category, filename, lineno, file=file, line=line)

return function


Expand All @@ -50,14 +54,12 @@ class Packager:
same version of OCDS.
"""

def __init__(self, force_version: Union[str, None] = None, ignore_version: bool = False):
def __init__(self, force_version: Union[str, None] = None):
"""
:param force_version: version to use instead of the version of the first release package or individual release
:param ignore_version: do not raise an error if the versions are inconsistent across items to merge
"""
self.package = _empty_record_package()
self.version = force_version
self.ignore_version = ignore_version

if USING_SQLITE:
self.backend = SQLiteBackend()
Expand All @@ -70,22 +72,27 @@ def __enter__(self):
def __exit__(self, type_, value, traceback):
self.backend.close()

def add(self, data):
def add(self, data, ignore_version: bool = False):
"""
Adds release packages and/or individual releases to be merged.
:param data: an iterable of release packages and individual releases
:param ignore_version: do not raise an error if the versions are inconsistent across items to merge
:raises InconsistentVersionError: if the versions are inconsistent across items to merge
"""
for i, item in enumerate(data):
version = get_ocds_minor_version(item)
if self.version:
if not self.ignore_version and version != self.version:
if not ignore_version and version != self.version:
# OCDS 1.1 and OCDS 1.0 have different merge rules for `awards.suppliers`. Also, mixing new and
# deprecated fields can lead to inconsistencies (e.g. transaction `amount` and `value`).
# https://standard.open-contracting.org/latest/en/schema/changelog/#advisories
raise InconsistentVersionError(f'item {i}: version error: this item uses version {version}, but '
f'earlier items used version {self.version}', self.version, version)
raise InconsistentVersionError(
f'item {i}: version error: this item uses version {version}, '
f'but earlier items used version {self.version}',
self.version,
version,
)
else:
self.version = version

Expand All @@ -106,17 +113,29 @@ def add(self, data):

self.backend.flush()

def output_package(self, merger, return_versioned_release=False, use_linked_releases=False, streaming=False):
def output_package(
self,
merger: ocdsmerge.merge.Merger,
return_versioned_release: bool = False,
use_linked_releases: bool = False,
streaming: bool = False,
convert_exceptions_to_warnings: bool = False,
):
"""
Yields a record package.
:param ocdsmerge.merge.Merger merger: a merger
:param bool return_versioned_release: whether to include a versioned release in each record
:param bool use_linked_releases: whether to use linked releases instead of full releases, if possible
:param bool streaming: whether to set the package's records to a generator instead of a list
:param merger: a merger
:param return_versioned_release: whether to include a versioned release in each record
:param use_linked_releases: whether to use linked releases instead of full releases, if possible
:param streaming: whether to set the package's records to a generator instead of a list
:param convert_exceptions_to_warnings: whether to convert inconsistent type errors from OCDS Merge to warnings
"""
records = self.output_records(merger, return_versioned_release=return_versioned_release,
use_linked_releases=use_linked_releases)
records = self.output_records(
merger,
return_versioned_release=return_versioned_release,
use_linked_releases=use_linked_releases,
convert_exceptions_to_warnings=convert_exceptions_to_warnings,
)

# If a user wants to stream data but can’t exhaust records right away, we can add an `autoclose=True` argument.
# If set to `False`, `__exit__` will do nothing, and the user will need to call `packager.backend.close()`.
Expand All @@ -131,13 +150,20 @@ def output_package(self, merger, return_versioned_release=False, use_linked_rele

yield self.package

def output_records(self, merger, return_versioned_release=False, use_linked_releases=False):
def output_records(
self,
merger: ocdsmerge.merge.Merger,
return_versioned_release: bool = False,
use_linked_releases: bool = False,
convert_exceptions_to_warnings: bool = False,
):
"""
Yields records, ordered by OCID.
:param ocdsmerge.merge.Merger merger: a merger
:param bool return_versioned_release: whether to include a versioned release in the record
:param bool use_linked_releases: whether to use linked releases instead of full releases, if possible
:param merger: a merger
:param return_versioned_release: whether to include a versioned release in the record
:param use_linked_releases: whether to use linked releases instead of full releases, if possible
:param convert_exceptions_to_warnings: whether to convert inconsistent type errors from OCDS Merge to warnings
"""
for ocid, rows in self.backend.get_releases_by_ocid():
record = {
Expand All @@ -163,18 +189,30 @@ def output_records(self, merger, return_versioned_release=False, use_linked_rele
with warnings.catch_warnings():
warnings.showwarning = _showwarning(showwarning, ocid)

record['compiledRelease'] = merger.create_compiled_release(releases)
if return_versioned_release:
record['versionedRelease'] = merger.create_versioned_release(releases)
try:
record['compiledRelease'] = merger.create_compiled_release(releases)
if return_versioned_release:
record['versionedRelease'] = merger.create_versioned_release(releases)
except InconsistentTypeError as e:
if convert_exceptions_to_warnings:
warnings.warn(MergeErrorWarning(e))
else:
raise

yield record

def output_releases(self, merger, return_versioned_release=False):
def output_releases(
self,
merger: ocdsmerge.merge.Merger,
return_versioned_release: bool = False,
convert_exceptions_to_warnings: bool = False,
):
"""
Yields compiled releases or versioned releases, ordered by OCID.
:param ocdsmerge.merge.Merger merger: a merger
:param bool return_versioned_release: whether to yield versioned releases instead of compiled releases
:param merger: a merger
:param return_versioned_release: whether to yield versioned releases instead of compiled releases
:param convert_exceptions_to_warnings: whether to convert inconsistent type errors from OCDS Merge to warnings
"""
for ocid, rows in self.backend.get_releases_by_ocid():
releases = (row[-1] for row in rows)
Expand All @@ -183,10 +221,16 @@ def output_releases(self, merger, return_versioned_release=False):
with warnings.catch_warnings():
warnings.showwarning = _showwarning(showwarning, ocid)

if return_versioned_release:
yield merger.create_versioned_release(releases)
else:
yield merger.create_compiled_release(releases)
try:
if return_versioned_release:
yield merger.create_versioned_release(releases)
else:
yield merger.create_compiled_release(releases)
except InconsistentTypeError as e:
if convert_exceptions_to_warnings:
warnings.warn(MergeErrorWarning(e))
else:
raise


# The backend's responsibilities (for now) are exclusively to:
Expand Down
2 changes: 1 addition & 1 deletion setup.cfg
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[metadata]
name = ocdskit
version = 1.1.12
version = 1.1.13
author = Open Contracting Partnership
author_email = data@open-contracting.org
license = BSD
Expand Down
57 changes: 55 additions & 2 deletions tests/test_combine.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,24 @@

import pytest
from ocdsextensionregistry import ProfileBuilder
from ocdsmerge.exceptions import DuplicateIdValueWarning
from ocdsmerge.exceptions import DuplicateIdValueWarning, InconsistentTypeError

from ocdskit.combine import compile_release_packages, merge, package_records
from ocdskit.exceptions import InconsistentVersionError, UnknownVersionError
from ocdskit.exceptions import InconsistentVersionError, MergeErrorWarning, UnknownVersionError
from tests import read

inconsistent = [{
"ocid": "ocds-213czf-1",
"date": "2000-01-01T00:00:00Z",
"integer": 1
}, {
"ocid": "ocds-213czf-1",
"date": "2000-01-02T00:00:00Z",
"integer": {
"object": 1
}
}]


def test_package_default_arguments():
data = [item for filename in ('realdata/record-package-1.json', 'realdata/record-package-2.json')
Expand Down Expand Up @@ -86,6 +98,47 @@ def data():
list(merge(data(), ignore_version=True)) # no error


@pytest.mark.parametrize('return_package', [True, False])
def test_merge_inconsistent_type(return_package):
def data():
for release in inconsistent:
yield {'releases': [release]}

with pytest.raises(InconsistentTypeError):
list(merge(data(), return_package=return_package))


@pytest.mark.parametrize('return_package,expected', [
(
True,
[
{
'uri': '',
'publisher': {},
'publishedDate': '',
'version': '1.1',
'records': [{'ocid': 'ocds-213czf-1', 'releases': inconsistent}],
},
],
),
(
False,
[],
),
])
def test_merge_inconsistent_type_convert_exceptions_to_warnings(return_package, expected):
def data():
for release in inconsistent:
yield {'releases': [release]}

with pytest.warns(MergeErrorWarning) as records:
output = list(merge(data(), return_package=return_package, convert_exceptions_to_warnings=True))

assert output == expected
assert len(records) == 1
assert str(records[0].message) == "ocds-213czf-1: An earlier release had the literal 1 for /integer, but the current release has an object with a 'object' key" # noqa: E501


def test_compile_release_packages():
with pytest.warns(DeprecationWarning) as records:
compiled_releases = list(compile_release_packages([]))
Expand Down

0 comments on commit 9b9c59c

Please sign in to comment.