Skip to content
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

Add a from_geo_json() to Transformer #7

Merged
merged 1 commit into from
Aug 28, 2024
Merged

Conversation

gjclark
Copy link
Contributor

@gjclark gjclark commented Aug 27, 2024

CIRRUS-NISAR requires generating Spatial Extent from GeoJSON and not WKT as before.

@gjclark gjclark changed the title Add a from_geo_json() to Transformer Add a from_geo_json() to Transformer Aug 27, 2024
@gjclark gjclark force-pushed the gjc/feature/from_geo_json branch 5 times, most recently from 3fc2ed8 to cac329c Compare August 27, 2024 22:47
@gjclark
Copy link
Contributor Author

gjclark commented Aug 27, 2024

def to_polygons(obj: Geometry) -> TransformationResult:
    """Convert a geometry to a sequence of polygons.
    :returns: a generator yielding the polygon sequence.
    :raises: Exception
    """
    if isinstance(obj, MultiPolygon):
        for poly in obj.geoms:
            yield poly
        return
    if isinstance(obj, Polygon):
        yield obj
        return
    raise Exception(f"WKT: '{obj}' is not a Polygon or MultiPolygon")

The Exception can be triggered by the GeoJSON as well. I feel like it is better to just remove WKT: so not to mislead when it's triggered by GeoJSON. obj itself is a shapely geometry anyways not a WKT.

@gjclark
Copy link
Contributor Author

gjclark commented Aug 27, 2024

Cannot for the life of me figure out the flake8 issue. Hashtag doesn't happen on my machine. But really, I don't know what's going on.

@gjclark gjclark requested review from reweeden and mattp0 August 27, 2024 22:55
@gjclark gjclark marked this pull request as ready for review August 27, 2024 22:55
tests/test_transformer.py Outdated Show resolved Hide resolved
Copy link
Contributor

@mattp0 mattp0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good minus the style stuff. We should fix that before its merged

@gjclark gjclark force-pushed the gjc/feature/from_geo_json branch from cac329c to af0fcbb Compare August 28, 2024 16:37
tests/test_transformer.py Outdated Show resolved Hide resolved
@gjclark gjclark force-pushed the gjc/feature/from_geo_json branch from af0fcbb to 48076a2 Compare August 28, 2024 17:29
@gjclark gjclark merged commit 51c291f into main Aug 28, 2024
5 checks passed
@gjclark gjclark deleted the gjc/feature/from_geo_json branch August 28, 2024 17:47
Copy link
Contributor

@reweeden reweeden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good except the indentation issue. I would support removing the WKT from the error message.

Comment on lines +198 to +199
Exception,
match=r"WKT: 'POINT \(30 10\)' is not a Polygon or MultiPolygon",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These lines are indented an extra level.

@reweeden
Copy link
Contributor

Oh, you also should've bumped the version number in the pyproject.toml. Now it is out of sync with the version on the releases page: https://github.com/asfadmin/geo_extensions/blob/main/pyproject.toml

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants