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 support for 4-dimensional coordinates to GeoJSON4STJ #139

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

Saracaen
Copy link

@Saracaen Saracaen commented Mar 22, 2024

Parsing geometry currently only supports reading up to 3-dimensional coordinates, even though there is a type (CoordinateZM) for 4-dimensional coordinates. This PR adds optional support for parsing 4-dimensional coordinates.

Parsing geometry currently only supports reading up to 3-dimensional coordinates, even though there is a type (CoordinateZM) for 4-dimensional coordinates. This commit adds optional support for 4-dimensional coordinates.
@CLAassistant
Copy link

CLAassistant commented Mar 22, 2024

CLA assistant check
All committers have signed the CLA.

@Saracaen Saracaen changed the title Add support for 4-dimensional coordinates Add support for 4-dimensional coordinates to GeoJSON4STJ Mar 22, 2024
Copy link
Member

@airbreather airbreather left a comment

Choose a reason for hiding this comment

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

From RFC 7946, section 3.1.1:

Implementations SHOULD NOT extend positions beyond three elements because the semantics of extra elements are unspecified and ambiguous. Historically, some implementations have used a fourth element to carry a linear referencing measure (sometimes denoted as "M") or a numerical timestamp, but in most situations a parser will not be able to properly interpret these values.

JTS and NTS coordinate sequences can carry values for any arbitrary number of spatial dimensions and "measures", so I cannot approve of supporting more than what is unambiguously defined by the spec unless it also comes with some kind of channel for either the caller (in code) or the producer (in the data stream itself) to tell us how to resolve ambiguities.

@Saracaen
Copy link
Author

Saracaen commented May 1, 2024

What kind of ambiguities would you expect to encounter? A sequence could still have more than four dimensions, but anything beyond that would still be ignored. I am assuming, perhaps naively so, that a sequence would at least always be consistent, in that if there is a 4th ordinate it would be always be in the same place. It would be weird for there to be an M ordinate on the 4th place in one coordinate, and in the other in the same sequence it is expected to be M3 for example.

And what would an acceptable solution look like? Would it be acceptable to have a property flag for this behavior, that is disabled by default and could be enabled via the GeoJsonConverterFactory? Provided that it has a clear name indicating the behavior, maybe something like ParseFourthOrdinateAsM or something along those lines.

@airbreather
Copy link
Member

What kind of ambiguities would you expect to encounter?

Speaking for myself and my own expectations, I would expect to encounter no ambiguities: a GeoJSON file with four-dimensional coordinates probably calls the coordinate in the fourth dimension "M".

But I'm trying to help maintain a library that builds a bridge between NTS and a published spec that explicitly recommends against doing exactly this for reasons of ambiguity (among others), in a world where such ambiguity can be shown to exist in NTS.

I am assuming, perhaps naively so, that a sequence would at least always be consistent, in that if there is a 4th ordinate it would be always be in the same place.

It's not the position of the fourth ordinate that's the problem, but its meaning. The first three are all defined by the spec to be, in order, [(longitude or easting), (latitude or northing), (altitude or elevation)] (where the third element does not always appear). Already, this rules out XYM sequences, which JTS and NTS can produce just fine.

The fourth ordinate, if present, explicitly does not have an interpretation or meaning in the spec, so it would be up to our implementation to define it. In doing so, I look to the existing solution and what we implemented in the transition from 1.x to 2.x, which included a major overhaul that happened to also add support for measures and other dimensions beyond the standard 3.

Per this overhaul, NTS coordinates and coordinate sequences (inherited from JTS) can support high numbers of spatial dimensions and measures, as long as there are always at least 2 spatial dimensions. This includes a combination, XYM, which is impossible to exactly represent in standard GeoJSON. See below for what can be done in NTS:

int ordIndex;

var xy = PackedCoordinateSequenceFactory.DoubleFactory.Create(new double[] { 11, 12, 21, 22, 31, 32 }, 2, 0);
Assert.That(!xy.TryGetOrdinateIndex(Ordinate.Z, out _));
Assert.That(!xy.TryGetOrdinateIndex(Ordinate.M, out _));

var xyz = PackedCoordinateSequenceFactory.DoubleFactory.Create(new double[] { 11, 12, 13, 21, 22, 23, 31, 32, 33 }, 3, 0);
Assert.That(xyz.TryGetOrdinateIndex(Ordinate.Z, out ordIndex) && ordIndex == 2);
Assert.That(!xyz.TryGetOrdinateIndex(Ordinate.M, out _));

// impossible to represent exactly with standard GeoJSON, but legal in JTS + NTS
var xym = PackedCoordinateSequenceFactory.DoubleFactory.Create(new double[] { 11, 12, 13, 21, 22, 23, 31, 32, 33 }, 3, 1);
Assert.That(!xym.TryGetOrdinateIndex(Ordinate.Z, out _));
Assert.That(xym.TryGetOrdinateIndex(Ordinate.M, out ordIndex) && ordIndex == 2);

var xyzm = PackedCoordinateSequenceFactory.DoubleFactory.Create(new double[] { 11, 12, 13, 14, 21, 22, 23, 24, 31, 32, 33, 34 }, 4, 1);
Assert.That(xyzm.TryGetOrdinateIndex(Ordinate.Z, out ordIndex) && ordIndex == 2);
Assert.That(xyzm.TryGetOrdinateIndex(Ordinate.M, out ordIndex) && ordIndex == 3);

// not common, but legal in JTS + NTS
var xyz_AndSpatial4 = PackedCoordinateSequenceFactory.DoubleFactory.Create(new double[] { 11, 12, 13, 14, 21, 22, 23, 24, 31, 32, 33, 34 }, 4, 0);
Assert.That(xyz_AndSpatial4.TryGetOrdinateIndex(Ordinate.Z, out ordIndex) && ordIndex == 2);
Assert.That(xyz_AndSpatial4.TryGetOrdinateIndex(Ordinate.Spatial4, out ordIndex) && ordIndex == 3);
Assert.That(!xyz_AndSpatial4.TryGetOrdinateIndex(Ordinate.M, out _));

Connecting this to GeoJSON, if we "just" define the fourth dimension to be M by convention, and we release with that, then if we want to add support for something with at least 4 spatial dimensions, we can only do so (in a backwards-compatible way) by requiring at least 5 dimensions per coordinate.

And what would an acceptable solution look like? Would it be acceptable to have a property flag for this behavior, that is disabled by default and could be enabled via the GeoJsonConverterFactory? Provided that it has a clear name indicating the behavior, maybe something like ParseFourthOrdinateAsM or something along those lines.

I would personally have no major problems with the signal coming as a new flag on GeoJsonConverterFactory, or perhaps two (dimension and measures, consistent with other coordinate sequence stuff), allowing the user to tell us the maximum number of dimensions that have meaning in their context, and how they should be labeled.

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