Skip to content
This repository has been archived by the owner on Jun 22, 2023. It is now read-only.

Remove (standalone) GeoAPI #70

Closed
airbreather opened this issue Mar 27, 2019 · 13 comments
Closed

Remove (standalone) GeoAPI #70

airbreather opened this issue Mar 27, 2019 · 13 comments
Labels
breaking change Doing this would break either source or binary compatibility with existing versions.
Milestone

Comments

@airbreather
Copy link
Member

airbreather commented Mar 27, 2019

After #68 (comment) I've been thinking hard about this, and the more I do, the more confident I feel in saying that standalone GeoAPI feels like a solution in search of a problem.

As I mentioned in that linked comment, maintaining GeoAPI as a standalone thing means maintaining a separate Git repository, complete with its own separate GitHub issue tracker and its own separate milestones, containing a separate Visual Studio solution that builds two separate NuGet packages exposing types in their own separate namespaces.

This is a lot of separate stuff we have to maintain, and users probably see it as an arbitrary division (frankly, I do too, every time I have to add more using statements). We also often have to update both together anyway when porting new JTS updates, which adds a bit of a burden from time to time. There does not seem to be enough value here to justify all this cost, and I have not yet seen a proposal which would add the value that I claim is missing.

Even if we do something like the IGeometryDefinition idea from #68 (comment), that bundle of interfaces seems equally fit in the NetTopologySuite.Core package, especially since moving the mathematically interesting capabilities off of the GeoAPI interfaces only seems to decrease the already low number of use cases that could be satisfied with a reference to the GeoAPI.Core package without also referencing the NetTopologySuite.Core package.

So I propose that we put this GitHub project in maintenance mode (keeping it on 1.x), move the useful types it exposes into other projects in the NetTopologySuite organization starting with version 2.x, and continue ongoing efforts to redesign the API over there.

It's very tempting to try to do some of that redesign as part of this, but I think this issue needs to be focused on just moving what we already have (warts and all), only adjusting namespaces at most.

I volunteer to write up the specific details of where everything should go, once there's a consensus on whether or not to do this at all, but for the most part (I'm sure there would be exceptions):

  • Things that go into the GeoAPI.Core package would go somewhere under this folder, which becomes the NetTopologySuite.Core package.
  • Things that go into the GeoAPI.CoordinateSystems package would go under this folder, which becomes the NetTopologySuite.CoordinateSystems package.

If we do this, it's important that we keep the ability to trace the history of the moved files... probably, the commit on the "moved-to" side should include "as of NetTopologySuite/GeoAPI@d2b5ecb" (or whatever commit) when it adds the files so that we can click that link to go back to the history of the file on this side.

@airbreather airbreather added the breaking change Doing this would break either source or binary compatibility with existing versions. label Mar 27, 2019
@airbreather airbreather added this to the v2.0 Candidate milestone Mar 27, 2019
@FObermaier
Copy link
Member

FObermaier commented Mar 27, 2019

Things that go into the GeoAPI.CoordinateSystems package would go under this folder, which becomes the NetTopologySuite.CoordinateSystems package.

Most of this belongs to ProjNet, and it is also a use case for GeoAPI:

  • IMathTransform.Transform(Coordinate),
  • IMathTransform.Transform(ICoordinateSequence)

Not that this could not be handled differently, though.

I also did think about some use-cases for geometry definition without actual topological functionality:

  • reprojection service
  • plain map rendering

The question however is, would the possibility to create ones own geometry definition ever be adopted by anyone or would everyone stick with NTS geometry.

@FObermaier
Copy link
Member

FObermaier commented Mar 28, 2019

After having thought about it a bit more, a candidate for a limited IGeometryDefinition might be the folks at DotSpatial. It could lift the burden of having to update their private copy of NetTopologySuite every once in a while.

Thats why I invite @danames and @jany-tenaj to comment.

@airbreather
Copy link
Member Author

airbreather commented Mar 28, 2019

  • plain map rendering

I did touch on SharpMap in the other comment:

  • SharpMap seems like it may be a candidate (I haven't looked at it in much detail). But...
    • Doesn't even that have to clip / ensure valid topology / do basic intersection testing?

  • reprojection service

Wouldn't you still need to ensure valid topology after the transformation in order to do it correctly?


The question however is, would the possibility to create ones own geometry definition ever be adopted by anyone or would everyone stick with NTS geometry.

This is what sent me off on this thread. I doubt it.


the folks at DotSpatial.

As it pertains to this specific issue, it looks like they are very much using the NTS algorithms (this is one of the first files that came up when I searched their repo for uses), so I don't think they'd be profoundly impacted if IGeometryDefinition lives in NetTopologySuite.Core.

As it pertains to #30, DotSpatial does look like the best candidate for IGeometryDefinition that I've seen so far: their changes all seem to be to add more consistent Z/M support, and although a lot of that is already in develop (with more coming once NetTopologySuite/NetTopologySuite#303 gets merged), their version of Envelope does have Z/M stuff that JTS's still does not (plus, of course, other changes to support setting the Z / M bounds on Envelope instances).

@danames
Copy link

danames commented Mar 28, 2019 via email

@jany-tenaj
Copy link

I have to say when I switched the code to NTS I was wondering what GeoAPI was good for. It felt senseless to have to go there to have to look there for some of the files while most where in NTS.

Not sure whether this IGeometryDefinition would be good for us. Basically I cloned your code to get M/Z into it, because when I tried to use our own ICoordinateArraySequence (if I remember correctly) for the geometries as @FObermaier had suggested, the problem was that whenever I gave those geometries to NTS it returned Nts geometries that no longer contained M/Z. So this was basically useless.

Because we did just a few changes I don't see a big problem with just updating the source code from time to time to stay up to date.

@FObermaier
Copy link
Member

Now, as there does not seem to be much support for standalone GeoAPI, let's do as @airbreather suggested:

  1. Put this Repository in maintenance mode
  2. Move GeoAPI classes/structs/interfaces to their respective projects (NTS/ProjNet)
  3. Eliminate interfaces that are not part of JTS, mainly IGeometry et. al.

@jany-tenaj
Copy link

@FObermaier If you remove IGeometry what will we get instead? Just asking because we use IGeometry quite a lot.

@DGuidi
Copy link
Contributor

DGuidi commented Apr 2, 2019

GeoAPI started as an attempt to make this in .net code, but faster I realized that the effort was too big for my time and knowledge, and I thinked that eventually can be useful to have an "interface layer" that hides actual NTS implementation, even if actually GeoAPI interfaces matches closely NTS implementations, just because of program to an interface, not to an implementation.

Your points are all valid, so if all of youy think GeoAPI it's only a complication, I of course don't have any objection.

@FObermaier
Copy link
Member

If you remove IGeometry what will we get instead? Just asking because we use IGeometry quite a lot.

You have the abstract Geometry class and the specific classes Point, LineString, Polygon, their Multi... variants and GeometryCollection.

@airbreather
Copy link
Member Author

airbreather commented Apr 3, 2019

@DGuidi thank you for providing the context and for challenging me to respond with more justification on this big change.

OGC GeoAPI 3.0.1 doesn't specify much that's relevant to NTS Core, but some relevant stuff it does specify is significantly different from what's currently in JTS.

Our Coordinate, for example, seems to match OGC GeoAPI's DirectPosition, which in OGC GeoAPI needs to be able to tell you its spatial reference (which is essentially a denormalized SRID).

This is a big deal on its own, but it goes even further when you look at some of the pending stuff.

So with regards to the parts of our GeoAPI.NET interfaces that came from JTS, in short, I get the feeling that we'd be better served waiting for those parts of the specification to be finalized and then (if there's demand for it) implementing the GeoAPI standard as a dedicated effort with a direct line of communication open with the OGC as we do so to ensure that we conform to the specification (and, perhaps, to offer our own feedback). In the meantime, it's a burden for a bunch of reasons I've mentioned.

re: the parts of OGC GeoAPI that our GeoAPI.NET implements more faithfully (coordinate reference / transformation stuff), I don't think that what we have is all that bad. That said, I have a guess that it's also hardly used independent of ProjNet4GeoAPI. Given that it's still not a complete port of the relevant parts of OGC GeoAPI 3.0.1, and given that the rest of OGC GeoAPI 3.0.1 is not likely to be implemented in .NET anytime "soon", I feel that the benefits of bringing GeoAPI.NET along as a standalone part of NTS v2 are not worth the ongoing costs.


  1. Put this Repository in maintenance mode
  2. Move GeoAPI classes/structs/interfaces to their respective projects (NTS/ProjNet)

I'll get started on this once NetTopologySuite/NetTopologySuite#303 is finished.

  1. Eliminate interfaces that are not part of JTS, mainly IGeometry et. al.

I'm not going to immediately jump on this, certainly not at the same time as absorbing GeoAPI.NET into its implementations. I didn't want this issue to get too bogged down by the concerns of #67 / #68, as those concerns are different (albeit related) and have their own costs / benefits.


If you remove IGeometry what will we get instead? Just asking because we use IGeometry quite a lot.

@jany-tenaj yeah I expect as much... #68 implies a non-trivial effort for most downstream folks to migrate from v1.x. I hope it doesn't get received as being too bad, but it's not something we should do lightly.

@airbreather
Copy link
Member Author

Most of this belongs to ProjNet, and it is also a use case for GeoAPI:
* IMathTransform.Transform(Coordinate),
* IMathTransform.Transform(ICoordinateSequence)

  1. Move GeoAPI classes/structs/interfaces to their respective projects (NTS/ProjNet)

NetTopologySuite.CoordinateSystems.Transformations.GeometryTransform is a reason I called out the NetTopologySuite.CoordinateSystems project... that could probably move into ProjNet4GeoAPI too?

@airbreather
Copy link
Member Author

Oops I was kinda hoping I couldn't entirely resolve this issue from a commit in a different repository, since there's still the coordinate system stuff.

@airbreather
Copy link
Member Author

  • renamed branch develop to v2-aborted/develop
  • renamed branch v1/develop to develop
  • renamed branch v1/master to master
  • unlisted all 2.x prereleases on NuGet

Looks like this is done.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
breaking change Doing this would break either source or binary compatibility with existing versions.
Projects
None yet
Development

No branches or pull requests

5 participants