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 ocean.json and test, which is currently failing #146

Closed

Conversation

alexgleith
Copy link

Related issues and pull requests

Description

Bounding box function is failing on a complex global geometry that does not overlap the antimeridian.

Checklist

  • Add tests
  • Add docs
  • Update CHANGELOG

@gadomski gadomski self-requested a review October 15, 2024 10:54
@gadomski
Copy link
Owner

@alexgleith the ocean.json that you provided seemed broken to me — it was a multipolygon with only one member, and the exterior shape looked like it was a second element of the polygon, not the first. I corrected it and the tests pass. Can you check my updated ocean.json (cdc7172) and tell me if I'm missing something?

@alexgleith
Copy link
Author

Oh damn, sorry.

I was using this data file, but it's a FeatureCollection, which isn't supported, so I attempted to manually make it a multipolygon and clearly failed.

https://github.com/opendatacube/odc-geo/blob/develop/odc/geo/data/ocean.geojson.xz

Creating a BBOX for that Ocean poly resulted in a small slice about the width of the Black Sea, which is the issue I was hoping to highlight.

@gadomski gadomski self-assigned this Oct 16, 2024
@gadomski
Copy link
Owner

Yeah, I think that ocean file is bad too. I extracted the shape with:

jq '.features[0].geometry' ~/Downloads/ocean.geojson > tests/data/output/ocean.json

Then visualized it with shapely:

import shapely.geometry
import json

with open("tests/data/output/ocean.json") as f:
    shape = json.load(f)
shapely.geometry.shape(shape)

ocean

@alexgleith
Copy link
Author

What's wrong with it?

It works in geopandas and QGIS.

@gadomski
Copy link
Owner

What's wrong with it?

Ah, nevermind, I messed up. The source file is fine, it's just a FeatureCollection with two features. When you made your multi-polygon, you put the first feature first, which is the Baltic Sea. If you grab the second feature, tests are fine:

jq '.features[1].geometry' ~/Downloads/ocean.geojson > tests/data/output/ocean.json

ocean

@alexgleith
Copy link
Author

Do you think the bbox should return a bbox of both, or should I be doing that somewhere else?

Currently it returns the X bounds for just the Baltic, and the Y bounds for the whole ocean, I think.

@gadomski
Copy link
Owner

🤔 not sure I agree:

$ jq '.features[0].geometry' ~/Downloads/ocean.geojson | antimeridian bbox  # the first feature is the Baltic
[46.6821, 36.7005, 54.7369, 47.0487]
$ jq '.features[1].geometry' ~/Downloads/ocean.geojson | antimeridian bbox  # the second feature is the whole oceaen
[-180.0, -85.609, 180.0, 90.0]

NB I'm using the as-of-yet unreleased bbox subcommand for the CLI, added here: #148

@alexgleith
Copy link
Author

Oh, I see... That's my facepalm!

from odc.geo.data import ocean_geom
from antimeridian import bbox

ocean = ocean_geom()
bbox = bbox(ocean)
bbox

[46.6821, -85.609, 54.7369, 90.0]

I expected the bbox of everything, but instead got the bbox of the first feature.

We can close this then, bbox should really be working per-feature and doing a bbox of a whole set of features is someone else's problem!

@gadomski gadomski closed this Oct 17, 2024
@alexgleith
Copy link
Author

Also: bbox(ocean.convex_hull) is correct, which is much simpler!

Thanks for your patience, Pete!

@alexgleith alexgleith deleted the add-failing-test-global-geom branch October 17, 2024 11:54
@gadomski
Copy link
Owner

Of course, no problem at all! Thanks for kicking the tires.

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.

2 participants