accept size parameter for polygonToCellsExperimental

[isaacbrodsky]

No description provided.

[coveralls]

coverage: 98.784% (+0.001%) from 98.783%
when pulling 5aa4ad2 on isaacbrodsky:experimental-polyfill-accept-size
into 7268098 on uber:master.

[ajfriend]

Some basic questions since I've forgotten some of our code organization practice:

• Should we add polygonToCellsExperimental to h3api.h.in instead of polyfill.h?
• Does H3_EXPORT serve a purpose outside of h3api.h.in, or does it only make sense in that file?

[isaacbrodsky]

* Should we add `polygonToCellsExperimental` to `h3api.h.in` instead of `polyfill.h`?

It depends on the level of support for this function. If this is an experimental function but we intend for bindings to mostly use the iterator approach, I don't think it needs to be there. If this is a supported API that has backwards compatibility guarantees it should be in h3api.h.

* Does `H3_EXPORT` serve a purpose outside of `h3api.h.in`, or does it only make sense in that file?

H3_EXPORT ensures that the symbol is visible outside of the library, and allows for it to be easily renamed.

[ajfriend]

I think the level of support is explained by the experimental.

But I also thought that symbols were external (e.g., supported API, "just the bindings", experimental functions) if and only if they were defined in h3api.h.in and use H3_EXPORT. And, as a result, anything defined in the other header files should be considered internal to the library.

Every symbol in the previous release followed this property. The (unreleased) polygonToCellsExperimental is the first set of functions outside of that convention.

Unless I'm misunderstanding something, I'm suggesting moving these to h3api.h.in.

[nrabinowitz]

This looks fine to me, with the caveat that if we're going to do this, we should plan to change the API again in the near future to return an iterator, so that callers could stream output iteratively into different memory blocks. If we're ok with that change later, this seems fine to ship early.

[isaacbrodsky]

This looks fine to me, with the caveat that if we're going to do this, we should plan to change the API again in the near future to return an iterator, so that callers could stream output iteratively into different memory blocks. If we're ok with that change later, this seems fine to ship early.

Agreed, I would like to expose an iterator to the bindings. Bindings can begin experimenting with the iterators, and for other uses inthe mean time, this will be a temporary experimental API to unblock the new version.

[ajfriend]

@nrabinowitz, do you mean change the API for polygonToCellsExperimental? Since it is "experimental", I think that's fine. But maybe it is worth flagging in the docstring that folks should expect that to happen?

[isaacbrodsky]

@nrabinowitz, do you mean change the API for polygonToCellsExperimental? Since it is "experimental", I think that's fine. But maybe it is worth flagging in the docstring that folks should expect that to happen?

I added a comment in h3api.h.in to that effect