search

Factual / geo

Clojure library for working with geohashes, polygons, and other world geometry
Eclipse Public License 1.0
301 stars 17 forks source link

Add Feature record, with :geometry and :properties #52

Closed willcohen closed 5 years ago

willcohen commented 5 years ago

The ad-hoc feature-map argument we added to geo.io never quite felt finished to me and has been a place where it's been easy for me to slip up and pass incorrectly defined maps to the functions. It seems like a perfect case for a clojure record, where we know the map will always need to have a geometry and properties field. @worace

worace commented 5 years ago

Makes sense to me. I haven't used records a ton in Clojure but it seems like kind of an underutilized feature TBH.

willcohen commented 5 years ago

After more thought, I'd like to let this sit a little while before merging. I'm still testing out how this would work in specific settings and I'm starting to question whether or not we should be adding new types to the library that don't provide new functionality in and of themselves. A piece of me is starting to think that this problem may be better solved in application-specific settings with something like spec. For this reason, it may be okay to go ahead with #46 and deal with this later.

willcohen commented 5 years ago

I'm going to close this for now after thinking about it some more. As a general rule, geo seems to maintain focus on geometry objects only (with the exception of reading to/from feature collections in geo.io), and it makes sense to keep it that way. After trying out some ideas, integrating full feature functionality will probably need its own protocol logic beyond Shapelike. I'm starting to put together a separate library that will wrap geo for geometries but will instead operate on feature-maps.