Return all chip geometries

[edurdevic]

Hello team,

I have added a parameter to configure the return of geometries for core chips.

Please do not spare any comment, this is a great learning opportunity for me ;)

[edurdevic]

UUps. I screwed something up when merging Master. On it

[edurdevic]

@milos-colic @sllynn I am getting an index out of bound exception that I don't understand.
Do you have any pointer about how to fix it? I will fix it, I just need a hint ;)

[sllynn]

Good to go, I think?

Thanks @edurdevic

[milos-colic]

looks good to me as well

[sllynn]

Looks great, thanks @edurdevic .

A couple of style things but otherwise love it.

[sllynn]

Oh - and let's update the python bindings / docs to reflect. @edurdevic are you happy to have a bash at that? If not, I'll happily take a look.

[milos-colic]

needs fixing copy call in withNewChildren in MosaicFill

[edurdevic]

@sllynn @milos-colic I updated all references to h3_mosaicfill, but I found many more references to h3_* in the docs.
I will make a separate PR to align all the namings across sql, py, Scala and docs ;)
I also fixed the copy issue.
I think it is good to go. Please take a fina look!

[milos-colic]

All good from my side!
@sllynn can you approve your "changes requested" and we can merge this.

[sllynn]

Do we still need to update the python bindings?

[edurdevic]

You are right @sllynn, I thought you intended bindings for the h3_ refactoring I did... But I am missing the keepCoreGoemetry parametr in python.
On it.

[edurdevic]

I have to fix the conflicts and also I noticed that mosaic_explode is missing some bindings, let me cover that with a test before we merge

[edurdevic]

@sllynn I am not able to understand why one of the new tests I created is failing.

I am passing a boolean parameter with expr("mosaic_explode(wkt, 4, true)") but it is interpreting it as expr("mosaic_explode(wkt, 4, 4)"). I don't understand where it is going wrong.

Could you please take a look?

[sllynn]

@sllynn I am not able to understand why one of the new tests I created is failing.

I am passing a boolean parameter with expr("mosaic_explode(wkt, 4, true)") but it is interpreting it as expr("mosaic_explode(wkt, 4, 4)"). I don't understand where it is going wrong.

Could you please take a look?

Hey @edurdevic, I think there was an issue with the way you were registering the SQL function. I think my latest commit fixed it, please take a look.

[sllynn]

OK, you'll need to update the python bindings to reflect the new set of arguments.

python/mosaic/api/functions.py#596

[edurdevic]

Wow, thanks for noticing the python bindings again @sllynn !
When I merged in master to fix the conflicts I reverted by mistake my changes to the bindings I did before... fc3041b#diff-9ec7293e119b7bae09d4eb204061160db4b37da62e85d6c4bd741e10d8551585

[edurdevic]

@sllynn The python bindings should be fine now. Please take a look

[milos-colic]

@edurdevic I have merged main into the PR to make sure the other PRs that were merged since this was opened do not break anything. If the build passes we can merge this one.

[milos-colic]

@sllynn this needs your approval since you requested changes in the past.
I checked and it looks like all of them were address, but if you dont mind could you give a quick check and approve?