Skip to content

Commit

Permalink
Fix an error with reprojected diffs in non-text output
Browse files Browse the repository at this point in the history
Non-text diff writers need to lookup CRS when doing reprojection,
and they do so by checking whether the CRS has changed in the diff.

When --no-sort-keys is given, it's possible for the meta deltas to be
lazy, and if so then the diff writer will fail to find the CRS in the
diff.

This change fixes the issue by making meta deltas always non-lazy, and
adds a regression test
  • Loading branch information
craigds committed Jan 9, 2025
1 parent 9350690 commit de81479
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 7 deletions.
14 changes: 7 additions & 7 deletions kart/diff_structs.py
Original file line number Diff line number Diff line change
Expand Up @@ -522,13 +522,13 @@ def iter_items(self):
for k, v in self._lazy_initial_contents:
yield (k, v)

# Invalidate this DeltaDiff; it's not safe to consume it again after this.
# `data` is the underlying contents of UserDict, which we inherit from.
# So overriding it to a non-dict will cause all dict methods to raise exceptions.
# > TypeError: argument of type 'InvalidatedDeltaDiff' is not iterable
self.data = InvalidatedDeltaDiff(
"DeltaDiff can't be used after iter_items() has been called"
)
# Invalidate this DeltaDiff; it's not safe to consume it again after this.
# `data` is the underlying contents of UserDict, which we inherit from.
# So overriding it to a non-dict will cause all dict methods to raise exceptions.
# > TypeError: argument of type 'InvalidatedDeltaDiff' is not iterable
self.data = InvalidatedDeltaDiff(
"DeltaDiff can't be used after iter_items() has been called"
)

def keys(self):
"""
Expand Down
25 changes: 25 additions & 0 deletions tests/test_diff.py
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,31 @@ def _check_geojson(featurecollection):
_check_geojson(odata["nz_pa_points_topo_150k"])


def test_diff_nontext_reprojection_no_sort_keys(data_archive, cli_runner):
"""
Regression test.
Non-text diff writers need to lookup CRS when doing reprojection, and they do so by
checking whether the CRS has changed in the diff.
When --no-sort-keys is given, it's possible for the meta deltas to be lazy, and if so then
the diff writer will fail to find the CRS in the diff.
This was fixed by making the meta deltas non-lazy. This test checks that it doesn't regress.
"""
with data_archive("points") as repo_path:
r = cli_runner.invoke(
[
"diff",
f"--output-format=json-lines",
"--output=-",
f"--crs=epsg:2193",
"--no-sort-keys",
"[EMPTY]...",
]
)

assert r.exit_code == 0, r.stderr


def test_show_crs_with_aspatial_dataset(data_archive, cli_runner):
"""
--crs should be ignored when used with aspatial data
Expand Down

0 comments on commit de81479

Please sign in to comment.