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

Separate monoidal part of strict layers #3330

Merged

Conversation

jisantuc
Copy link
Contributor

@jisantuc jisantuc commented Dec 30, 2020

Overview

This PR separates the monoidal part of StrictLayers -- the separate collections of geometries -- from the... I guess "metadata", you know, the other part.

It's useful, e.g., if you're getting geometries from somewhere and trying to combine everything into a vector tile layer. The collections of geometries form a lawful monoid, but StrictLayer as it's currently implemented does not.

Checklist

  • ./CHANGELOG.md updated, if necessary. Link to the issue if closed, otherwise the PR.
  • Module Hierarchy updated, if necessary
  • docs guides update, if necessary
  • New user API has useful Scaladoc strings
  • Unit tests added for bug-fix or new feature -- already tested, just a small reorganization

Demo

I would have used this here instead of hand-rolling a special Monoid for strict layer for each z/x/y triple

Notes

I'd like to prove this is a lawful monoid with discipline -- e.g. https://github.com/raster-foundry/raster-foundry/blob/develop/app-backend/datamodel/src/test/scala/ScopeSpec.scala#L126. It's pretty easy, but it would require new testing deps and also Arbitrary instances for everything going up the dependency chain from the MVTFeatures, which seems like a lot for what is in practice a very fancy Map[String, Seq[A]].

@jisantuc jisantuc changed the title separate monoidal part of strict layers Separate monoidal part of strict layers Dec 30, 2020
@pomadchin
Copy link
Member

pomadchin commented Dec 31, 2020

Hey James! Thanks for the PR; we haven't had a chance to look into it though.

One minor thing here, could you sign ECA please to make Eclipse license checker happy?

After that you'd need to add a sing-off footer into the commit via git commit --amend -s && git push origin -f feature/js/separate-monoidal-part-of-vt
If you have more than a single commit in the PR than all commit messages should have a sign off fitter

P.S. git commit --amend -s adds the necessary footer into the commit message that is required for the successful license check.

P.P.S. gz with your first official PR 🎉 🎉

@jisantuc jisantuc force-pushed the feature/js/separate-monoidal-part-of-vt branch from 37e82d5 to e60ff8e Compare January 4, 2021 15:20
@jisantuc jisantuc requested a review from echeipesh January 22, 2021 19:41
Signed-off-by: James Santucci <[email protected]>
@pomadchin pomadchin merged commit 431e8a6 into locationtech:master Jan 28, 2021
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