-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Fix --boundary crash on fiona 1.10 (CRSError: Invalid PROJ string: EPSG:4326) #2040
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.
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
smathermather
merged 6 commits into
OpenDroneMap:master
from
EmirTD:fix-boundary-crs-fiona110
Jul 3, 2026
+80
−1
Merged
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
a18d385
Fix --boundary crash on fiona 1.10 (CRSError: Invalid PROJ string: EP…
EmirTD 065ff95
Add regression test for load_boundary GeoJSON reprojection (#2039)
EmirTD 6fc2d24
Honor source CRS instead of assuming WGS84
EmirTD a80df8e
Add test that load_boundary honors non-WGS84 source CRS
EmirTD 4b4fe4e
Remove duplicated test block in test_boundary.py
EmirTD cbf0528
Remove redundant main check in test_boundary.py
EmirTD File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,126 @@ | ||
| import unittest | ||
|
|
||
| from opendm.boundary import load_boundary | ||
|
|
||
|
|
||
| class TestBoundary(unittest.TestCase): | ||
| def setUp(self): | ||
| # A simple WGS84 (EPSG:4326) GeoJSON polygon | ||
| self.boundary = { | ||
| 'type': 'Feature', | ||
| 'properties': {'name': 'boundary'}, | ||
| 'geometry': { | ||
| 'type': 'Polygon', | ||
| 'coordinates': [[ | ||
| [-91.99544, 46.84260], | ||
| [-91.99417, 46.84260], | ||
| [-91.99417, 46.84337], | ||
| [-91.99544, 46.84337], | ||
| [-91.99544, 46.84260], | ||
| ]] | ||
| } | ||
| } | ||
|
|
||
| def test_load_boundary_without_reprojection(self): | ||
| # Without a target CRS the coordinates are returned unchanged | ||
| coords = load_boundary(self.boundary) | ||
| self.assertEqual(len(coords), 5) | ||
| self.assertAlmostEqual(coords[0][0], -91.99544, places=5) | ||
| self.assertAlmostEqual(coords[0][1], 46.84260, places=5) | ||
|
|
||
| def test_load_boundary_reprojects_to_utm(self): | ||
| # Regression test for https://github.com/OpenDroneMap/ODM/issues/2039 | ||
| # On fiona >= 1.10, fiona.crs.to_string() returns "EPSG:4326" instead of a | ||
| # PROJ4 string, which made CRS.from_proj4() raise | ||
| # "CRSError: Invalid PROJ string: EPSG:4326". load_boundary must reproject | ||
| # a WGS84 GeoJSON boundary without raising. | ||
| utm15n = '+proj=utm +zone=15 +datum=WGS84 +units=m +no_defs' | ||
| coords = load_boundary(self.boundary, utm15n) | ||
| self.assertEqual(len(coords), 5) | ||
| # Reprojected coordinates are in meters (UTM), not degrees | ||
| for x, y in coords: | ||
| self.assertGreater(abs(x), 1000) | ||
| self.assertGreater(abs(y), 1000) | ||
|
|
||
| def test_load_boundary_honors_non_wgs84_source_crs(self): | ||
| # GeoJSON may declare a CRS other than WGS84 (allowed by RFC 7946 and | ||
| # written in practice by tools like QGIS). load_boundary must reproject | ||
| # from the file's declared CRS, not assume EPSG:4326. | ||
| boundary_utm = { | ||
| 'type': 'FeatureCollection', | ||
| 'crs': {'type': 'name', | ||
| 'properties': {'name': 'urn:ogc:def:crs:EPSG::32635'}}, | ||
| 'features': [{ | ||
| 'type': 'Feature', | ||
| 'properties': {}, | ||
| 'geometry': { | ||
| 'type': 'Polygon', | ||
| 'coordinates': [[ | ||
| [500000, 4540000], | ||
| [501000, 4540000], | ||
| [501000, 4541000], | ||
| [500000, 4541000], | ||
| [500000, 4540000], | ||
| ]] | ||
| } | ||
| }] | ||
| } | ||
| wgs84 = '+proj=longlat +datum=WGS84 +no_defs' | ||
| coords = load_boundary(boundary_utm, wgs84) | ||
| self.assertEqual(len(coords), 5) | ||
| # EPSG:32635 (UTM 35N) easting 500000 / northing ~4540000 is ~27E, ~41N. | ||
| # If the source CRS were wrongly assumed to be WGS84, these would not land here. | ||
| for lon, lat in coords: | ||
| self.assertTrue(26 < lon < 28, "unexpected lon: %s" % lon) | ||
| self.assertTrue(40 < lat < 42, "unexpected lat: %s" % lat) | ||
|
|
||
|
|
||
| if __name__ == '__main__': | ||
| unittest.main() | ||
| import unittest | ||
|
|
||
| from opendm.boundary import load_boundary | ||
|
|
||
|
|
||
| class TestBoundary(unittest.TestCase): | ||
| def setUp(self): | ||
| # A simple WGS84 (EPSG:4326) GeoJSON polygon | ||
| self.boundary = { | ||
| 'type': 'Feature', | ||
| 'properties': {'name': 'boundary'}, | ||
| 'geometry': { | ||
| 'type': 'Polygon', | ||
| 'coordinates': [[ | ||
| [-91.99544, 46.84260], | ||
| [-91.99417, 46.84260], | ||
| [-91.99417, 46.84337], | ||
| [-91.99544, 46.84337], | ||
| [-91.99544, 46.84260], | ||
| ]] | ||
| } | ||
| } | ||
|
|
||
| def test_load_boundary_without_reprojection(self): | ||
| # Without a target CRS the coordinates are returned unchanged | ||
| coords = load_boundary(self.boundary) | ||
| self.assertEqual(len(coords), 5) | ||
| self.assertAlmostEqual(coords[0][0], -91.99544, places=5) | ||
| self.assertAlmostEqual(coords[0][1], 46.84260, places=5) | ||
|
|
||
| def test_load_boundary_reprojects_to_utm(self): | ||
| # Regression test for https://github.com/OpenDroneMap/ODM/issues/2039 | ||
| # On fiona >= 1.10, fiona.crs.to_string() returns "EPSG:4326" instead of a | ||
| # PROJ4 string, which made CRS.from_proj4() raise | ||
| # "CRSError: Invalid PROJ string: EPSG:4326". GeoJSON is always WGS84 | ||
| # (RFC 7946), so load_boundary must reproject without raising. | ||
| utm15n = '+proj=utm +zone=15 +datum=WGS84 +units=m +no_defs' | ||
| coords = load_boundary(self.boundary, utm15n) | ||
| self.assertEqual(len(coords), 5) | ||
| # Reprojected coordinates are in meters (UTM), not degrees | ||
| for x, y in coords: | ||
| self.assertGreater(abs(x), 1000) | ||
| self.assertGreater(abs(y), 1000) | ||
|
|
||
|
|
||
| if __name__ == '__main__': | ||
| unittest.main() | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to be a duplicate of the above?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the duplicated test block, thanks for catching it.