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

Update the Collection extent to be a 2d array to match the specification #175

Merged
merged 2 commits into from
Jan 28, 2025

Conversation

CoryBianco
Copy link
Contributor

@CoryBianco CoryBianco commented Jan 28, 2025

I am testing with the OGCFeatureLayer from arcgis Core v4.30.9 and noticed that I was unable to get the features to display on the map. After looking through their library, they are expecting the bbox to be a 2d array. Thus when it tries to pull the bbox and sees that it is a 1d array, it tries to access a float and then errors out.

I updated the type of extent to be a 2d array instead of a 1d array and put the same

Code from the OGCFeatureLayer in arcgis Core v4.30.9

function K(e2) {
  var _a;
  const t = (_a = e2.extent) == null ? void 0 : _a.spatial;
  if (!t) return null;
  const n2 = t.bbox[0], i3 = 4 === n2.length, [r, o2] = n2, s2 = i3 ? void 0 : n2[2];
  return { xmin: r, ymin: o2, xmax: i3 ? n2[2] : n2[3], ymax: i3 ? n2[3] : n2[4], zmin: s2, zmax: i3 ? void 0 : n2[5], spatialReference: f.WGS84.toJSON() };
}

@CoryBianco CoryBianco changed the title Update the extent to be a 2d array Update the extent to be a 2d array instead of a regular array Jan 28, 2025
@CoryBianco CoryBianco changed the title Update the extent to be a 2d array instead of a regular array Update the extent to be a 2d array instead of a 1d array Jan 28, 2025
@CoryBianco
Copy link
Contributor Author

@dr-jts Hey there! I was curious if you had seen any issue like this before?

@dr-jts
Copy link
Collaborator

dr-jts commented Jan 28, 2025

Hey there! I was curious if you had seen any issue like this before?

No. But I don't use ArcGIS.

Is this valid according to the OGC spec?

internal/service/util.go Outdated Show resolved Hide resolved
@dr-jts dr-jts added the bug Something isn't working label Jan 28, 2025
@dr-jts
Copy link
Collaborator

dr-jts commented Jan 28, 2025

This does look like it's not implemented according to the OGC Features specification. The definition of theextent property in the JSON for collections is:

The definition allows multiple extents to be provided for a collection, with the first one being the overall extent (which allows systems to choose to read just one extent). (OGC Features Core issue 518 discusses this.) pg_featureserv only outputs a single extent.

It's very surprising this bug hasn't turned up until now!

Is this really all the changes needed to fix this? Does the UI still work?

@dr-jts dr-jts changed the title Update the extent to be a 2d array instead of a 1d array Update the Collection extent to be a 2d array instead of a 1d array Jan 28, 2025
@CoryBianco
Copy link
Contributor Author

CoryBianco commented Jan 28, 2025

This does look like it's not implemented according to the OGC Features specification. The definition of theextent property in collections is:

It's very surprising it hasn't turned up until now!

Is this really all the changes needed to fix this? Does the UI still work?

The UI actually did not work with it in a 1D array but it did work in a 2D array
image

just a pic of it working with the 2D array

@CoryBianco CoryBianco requested a review from dr-jts January 28, 2025 21:34
@dr-jts dr-jts merged commit 7a7b3fc into CrunchyData:master Jan 28, 2025
3 checks passed
@dr-jts
Copy link
Collaborator

dr-jts commented Jan 28, 2025

Also added a test to confirm the Extent structure: cd3ae95

@dr-jts dr-jts changed the title Update the Collection extent to be a 2d array instead of a 1d array Update the Collection extent to be a 2d array to match the specification Jan 28, 2025
@CoryBianco CoryBianco deleted the update-extent branch January 29, 2025 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants