Planned changes for 4.0.0

[isaacbrodsky]

This is a tracking issue for any breaking changes that we plan to include in the next major version.

• Rename kRing and remove hexRange, hexRanges, and maybe hexRing. (naming "k-ring" #42)
• Change all functions that allocate blocks of memory or that could receive invalid input to return a success/error code (OOM handling in C h3-java#10, JVM crash on large polyfills h3-java#21)
  ** Error codes should have meaning, e.g. invalid was not a valid H3 index, etc. Use core 3.1.0 and add h3Distance h3-java#22
• Consider accepting an allocator/free function for functions that must allocate memory (Use calloc for potentially large user-defined inputs #100)

[willcohen]

Do you all have an idea of what kRing and kRingDistances will get renamed to, or when you expect to decide (I see a range of options in #42)? We're working on adding H3 to a geospatial library in Clojure (Factual/geo#22), and before that integration gets released it'd be ideal to use whatever names you expect to use in the next major version so we don't immediately have to start aliasing functions there too.

[isaachier]

@willcohen can't you just reuse the Java API. I played with Clojure ~2 years ago and JVM integration was pretty straightforward.

[willcohen]

Absolutely! If picking replacement names for kRing and kRingDistances is a ways off, it's definitely workable to similarly alias the old names when the time comes, but I was just asking to see if we could avoid deprecating things downstream right off the bat especially if the change is coming soon.

Beyond being a pure wrapper, there's a little bit happening in the library beyond pure calls to Java to use clojure protocols to remove unnecessary reflection in clojure for Strings versus Longs, and to avoid having to call unidiomatically to H3Core's newInstance, but it tries to stick to the Java API and uses similar naming (k-ring for .kRing, etc.). The other main thing it does is use a clojure protocol to add interoperability between H3 GeoCoords and coordinate objects in JTS, spatial4j, and geohash, which facilitates polyfills on objects from those libraries as well as conversion of h3 indices or multipolygon coordinate sets directly into JTS objects (and, consequently, geojsons, wkts, and other outputs from there).

[jogly]

thoughts on changing H3ToParent and H3ToChildren to use relative resolution instead of absolute?

getting parent:
H3ToParent(h, GetResolution(h)-1) -> H3ToParent(h, 1)
H3ToChildren(h, GetResolution(h)+1) -> H3ToChildren(h, 1)

the name of the functions already imply direction, so a resolution of 0 is the identity and negative offsets are invalid.

[nrabinowitz]

thoughts on changing H3ToParent and H3ToChildren to use relative resolution instead of absolute?

Not sold - I think the relative use case is no more common than the absolute, and not worth introducing confusion. But if we wanted to introduce h3ToRelativeParent as a convenience I'd support.

[sahrk]

I agree with @nrabinowitz, I think in general both use cases are roughly as common.

[nrabinowitz]

I think another major-version change to consider is whether we can replace GeoPolygon and friends with LinkedGeoPolygon and friends. I really don't like having two different sets of data structures for this; the challenge is that GeoPolygon, while replaceable, is probably much faster than the linked version.

[jogly]

@nrabinowitz + @sahrk : i cannot anticipate the use case popularity for either relative or absolute and am not suggesting one is more popular than another. I do believe that h3To[Parent|Children] inherently suggests relative operations, though. while using h3, I found using absolute resolutions for each function counter-intuitive. if there isn't general agreement on this being counter-intuitive then I'm all-in for not changing the meaning of the function for my sake :)

[sahrk]

@joegilley I can see framing problems using either relative or absolute resolution; though I personally would also be more inclined to use relative, I don't think it's counter-intuitive to use absolute. Coincidently I was just using this function when you posted and it didn't feel odd. So I guess I'd still vote for both options. The question then is whether it warrants breaking the api to make the default the relative case.

[nmandery]

Looking at the planned changes it would be great to have defines for the version number in the h3api.h header file. For example something like

#define H3_VERSION_MAJOR 4
#define H3_VERSION_MINOR 0
#define H3_VERSION_PATCH 0

would really help projects depending on this library to deal with the API-change.

[isaacbrodsky]

@nmandery I think we could include that in the current version, since it's not changing an existing API.

[nmandery]

@isaacbrodsky That would also be great. For me it would just be important to have a method to check the H3 version when the API break happens.

[nrabinowitz]

Per offline discussion: We should have a consistent, documented approach to input validation in V4, including what the criteria are to add input validation to a function, how errors are returned, and what kind of benchmark changes would lead us to avoid validation in favor of performance.

[isaacbrodsky]

I've created a branch 4.x which can be used for developing the 4.0.0 release. PRs can be made against that branch for breaking changes, and when ready it can be merged into master of uber/h3.

[isaacbrodsky]

To try to encourage more progress on changes needed for 4.0.0, we're going to try removing the 4.x branch and instead develop 4.0.0 directly on master. Changes like bug fixes that also need to be applied to the stable 3.x release of H3 can be made on a branch if they're urgently needed before 4.0.0 is ready.

We have a Github project tracking what's in scope for 4.0.0: https://github.com/uber/h3/projects/1 We can continue discussion on those issues, and would welcome pull requests that propose solutions for them. I summarize the things we want to address for 4.0.0 as:

• API renaming, including naming of functions (kRing, hexRange, experimental functions, etc...) and types (consistent naming between the terms cells, index, etc).
• Adding error codes / return codes, and documenting input validation strategy.
• User specified allocation functions (minimally via #define, but this is an opportunity to replace the allocator programmatically.)
• Supporting different modes in polyfill. (Not sure if there's a proposal for this)
• And of course any other proposed changes that come up.

Right now we still do not expect to make breaking changes to the H3 index format for 4.0.0.

[isaacbrodsky]

We'll keep track of this using the project, https://github.com/uber/h3/projects/1 instead of this issue. Keep an eye out for H3 v4 in the future :)