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

turf-next proposal #306

Closed
tmcw opened this issue Feb 13, 2016 · 13 comments
Closed

turf-next proposal #306

tmcw opened this issue Feb 13, 2016 · 13 comments

Comments

@tmcw
Copy link
Collaborator

tmcw commented Feb 13, 2016

opinion warning: this is opinion

Turf 3.0 should

  • Switch to a uni-repo method. All core modules will be in a single repo. Addons will be managed as such
  • Turf will have a global dependency to lodash 4.x, which will help with some of the fp stuff but more importantly with immutability
  • Turf packages will still be bundled as split-out packages, like lodash modules or babel modules, and we'll take advantage of lerna to do that.
  • Turf support will be moved 100% to gis stackexchange or stackoverflow and off of github. This leaves GH as only the place for technical discussion

cc @Turfjs/owners

@mourner
Copy link
Contributor

mourner commented Feb 14, 2016

Support everything except the lodash dependency, which I find unnecessary.

@tmcw
Copy link
Collaborator Author

tmcw commented Feb 14, 2016

@mourner i'm fine with an alternative to lodash: mainly I'm excited by the inclusion of lodash-fp, which provides immutable methods. I think turf's mutability problems need a generalized solution. lodash is one, there are surely others.

@anandthakker
Copy link

Re: immutability -- I've been trying out seamless-immutable in a recent project, and I've been 80% happy with it, but my guess is that there would be perf problems out of the box

@mourner
Copy link
Contributor

mourner commented Feb 15, 2016

@tmcw although I don't have a lot of experience with immutability to have a weighted opinion here, I feel like immutability should be more of convention rather than a library you depend on (especially a bloated one like lodash) , at least in libraries like Turf as opposed to end-user applications.

I like the approach of seamless-immutable — it basically throws helpful exceptions when you mutate objects/array that should be immutable, but you can switch to "production build" which doesn't do that and performs much better.

@anandthakker
Copy link

I like the approach of seamless-immutable — it basically throws helpful exceptions when you mutate objects/array that should be immutable, but you can switch to "production build" which doesn't do that and performs much better.

@mourner Yeah I like this approach too. My concern using the actual lib with turf is that for each turf function you might have to do a deep traversal on input (to make input geojson immutable) and again on output (to make it back into a plain object), which I'd think would be noticeable overhead.

I guess that overhead could be avoided if turf were to return seamless-immutable objects, which look and act just like plain objects (especially using the production build), except for having a private, non-enumerable __immutable_invariants_hold property on them that's used internally by seamless-immutable.

@tmcw
Copy link
Collaborator Author

tmcw commented Feb 17, 2016

cc @morganherlocker @tcql what say you?

@tmcw
Copy link
Collaborator Author

tmcw commented Feb 17, 2016

Here's a proposed module roadmap:

turf v3 layout

  • geometry-helpers these modules should be one module
    • turf-polygon
    • turf-featurecollection
    • turf-point
    • turf-linestring
  • turf-statistics
  • turf-grid
    • turf-hex-grid
    • turf-point-grid
    • turf-square-grid
    • turf-triangle-grid
  • turf-operations
    • turf-union
    • turf-intersect: should be named intersection, to match union
    • turf-concave
    • turf-tag
    • turf-convex
    • turf-buffer
    • turf-simplify
    • turf-difference
    • turf-midpoint
    • turf-kinks
    • turf-centroid
    • turf-within
    • turf-center
    • turf-along
    • turf-square
    • turf-planepoint
    • turf-point-on-line
    • turf-point-on-surface
    • turf-line-slice
    • turf-extent
  • turf-analysis
    • turf-area
    • turf-inside
    • turf-bearing
    • turf-destination
    • turf-nearest
    • turf-line-distance
    • turf-distance
  • turf-geojson-utils
    • turf-combine
    • turf-flip
    • turf-random
    • turf-bezier
    • turf-explode: rename to something more descriptive, like turf-coordinates
    • turf-bbox-polygon
  • turf-interpolation
    • turf-isolines
    • turf-tin

rename

  • turf-size rename this turf-resize or something. turf-size is not a good name

deprecate

  • turf-merge: this is confusingly named and doesn't provide any value over calling union repeatedly
  • turf-envelope: does not provide any value over calling bboxPolygon(extent(features))
  • turf-quantile: provides no value over ss.quantile(features.map(f => f.properties.foo), [1, 2 3])
  • turf-jenks: provides no value over ss.jenks(features.map(f => f.properties.foo), 3)
  • turf-filter: provides no value over features.filter(f => f.properties.foo == 'bar')
  • turf-remove: provides no value over features.filter(f => f.properties.foo !== 'bar')
  • turf-reclass: provides no value over features.map(f => { class: getClass(f.properties), ...f.properties })

@morganherlocker
Copy link
Member

Thanks for posting this @tmcw. I think a modest v3 release that helps v3+n easier is a terrific idea.

high level

Switch to a uni-repo method. All core modules will be in a single repo. Addons will be managed as such

Turf packages will still be bundled as split-out packages, like lodash modules or babel modules, and we'll take advantage of lerna to do that.

👍 I think this would be drastically easier to maintain and release. The lodash approach that has evolved is great. Ideally, we would have one repo to develop under and publishing would release all the submodules and the master module as if they were separate (my understanding of how lodash works). This would make things like updating benchmarks, linting changes, testing core updates across internal dependencies a breeze compared to how it is now.

Turf will have a global dependency to lodash 4.x, which will help with some of the fp stuff but more importantly with immutability

I think we should design a pattern for how we do immutability. I do not think we should depend on the full lodash, but something more focused to deal specifically with immutability (if we need a library at all, instead of simply a codified pattern). A short note on best practices with respect immutability in the CONTRIBUTING.md file would go a long way in my opinion.

Turf support will be moved 100% to gis stackexchange or stackoverflow and off of github. This leaves GH as only the place for technical discussion

👎 I do not see any reason to do this. I have absolutely zero problem with support type questions here, and we have been able to help many people over the years through Github. Issues are cheap, and I honestly enjoy answering the questions. 😄

rename

turf-size rename this turf-resize or something. turf-size is not a good name

👍 resize is better. I would also be fine with dropping this entirely. I tend to be using different patterns these days around this type of operation (explicit distance-based bbox buffers).

deprecate

👍 I agree with all of these except for turf-envelope, which I think is a useful convenience method. A method that saves a bit of thought and typing for quick scripting is worthwhile IMO. The other modules on the list are mostly clunky wrappers around ss, and it makes sense to encourage people to dig a bit deeper. turf-merge is a performance landmine many people have run into, and I think we would have less issues around this if people had to explicitly say "yep, I'm unioning a billion polygons YOLO".

layout

Are you proposing to namespace the actual library, or simply organize the code this way? I prefer the flat structure we currently have, since remembering a module name is much easier than remembering a module name + which category it belongs to. This is a big pain point in similar stats and spatial libraries in my experience, and the flat structure lines up with other "easy to use" grab bag libraries like lodash.

@tmcw
Copy link
Collaborator Author

tmcw commented Feb 18, 2016

Re: layout. I think each category should be a module, and the modules to be reflected in namespaces. So like turf.geom.featurecollection and turf.grid.hex. And you'd install turf-geom to get just the geometry helpers, and turf-grid to get just the grids.

And, I know I'm in the minority here, but I still really want to get rid of geometry helpers :)

@tmcw tmcw mentioned this issue Feb 23, 2016
4 tasks
tmcw added a commit that referenced this issue Feb 27, 2016
Remove turf-jenks and turf-quantile. Refs #306
@tmcw
Copy link
Collaborator Author

tmcw commented Feb 27, 2016

The actions that had consensus are now in master.

@tmcw tmcw closed this as completed Feb 27, 2016
tmcw added a commit that referenced this issue Apr 24, 2016
Refs #306

> turf-size rename this turf-resize or something. turf-size is not a good name
> 👍 resize is better. I would also be fine with dropping this entirely. I
> tend to be using different patterns these days around this type of operation
> (explicit distance-based bbox buffers).
@tmcw tmcw reopened this Apr 26, 2016
@tmcw
Copy link
Collaborator Author

tmcw commented Apr 26, 2016

Still need to remove turf-reclass

tmcw added a commit that referenced this issue Apr 26, 2016
@tmcw
Copy link
Collaborator Author

tmcw commented May 3, 2016

Fixed in f9e1bd9

@tmcw tmcw closed this as completed May 3, 2016
benstoltz added a commit to benstoltz/turf that referenced this issue May 30, 2016
* Remove moot `version` property from bower.json

Per bower/spec@a325da3

Also their maintainer says they probably won't ever use it: http://stackoverflow.com/questions/24844901/bowers-bower-json-file-version-property

* 📝 fix broken version badge

reformat badge markdown for readability

* 📝 erase -> difference

* Updated package.json to include latest published npm turf-buffer version

* link to turfjs-builder app

* Remove turf.merge

* Add upgrade notes to CHANGELOG

* Remove turf_modules dir, merge from README

* Unirepo: turf in one repository

* Update turf-flip

* Bootstrap before test

* Port isolines updates

* Remove turf-filter and turf-remove

* Add multiLineString to helpers. Fixes Turfjs#147

* Add multipoint. Fixes Turfjs#146

* Centralize lint, fix first slew of issues

* Finish eslint sweep

* Use jscs for better auto-indentation

* Merge turf-feature into turf-helpers

* Condense turf statistics sprawl into turf-collect

* Remove turf-jenks and turf-quantile. Refs Turfjs#306

* Update changelog for removals

* Update minified file link

The previous link was returning a file not found, repo seems to be gone. I've now linked to the same version being used on https://turfjs-builder.herokuapp.com/

* Update README.md

v2.0.2.

* Port turf-junkyard/turf-point-on-line#4

* Port improvements from child projects

* Add multiPolygon and geometryCollection to helpers

* Port turf-junkyard/turf-convex#8

* Add tesselate module

* Improve performance by using invariant.getCoord

This simplifies calls to methods that previously required GeoJSON:
by allowing coordinates as well in those calls, we cut down on churning
through objects.

This needs a second pass to hook up dependencies and standardize
invariant once I land.

* Fix tests

* Use temp fork of lerna

* Bump node

* Avoid double-bootstrap

* Remove dox, fix bearing dep

* Fix typo Turfjs/turf-distance#14

* Update CONTRIBUTING.md

* Rename turf-extent to turf-bbox

* add inches, yards, metres units to distance function...unsure if configured correctly

* Unquote non-quoted object properties https://twitter.com/tmcw/status/723374287537606656. Also update to lerna 2.0.0-beta

* add missing semi-colon

* Mass simplifications.

* move stuff to test
* simplify code
* remove bad use of 'set' in a bunch of places. we aren't dealing with
  sets.
* ugh the family in front of me doesn't understand timezones and speaks
  at 80db

* Finish turf-combine rethink

* Improve line-slice docs.

* Document, fix, and test geometry compatibility in turf-line-slice

* Fix indentation, clean up triangle grid

* Remove tape and benchmark from dependencies

* Port turf-junkyard/turf-hex-grid#18 (Turfjs#356)

* Port turf-junkyard/turf-hex-grid#18

* Fix eslint

* eslint again

* Abolish turf-size

Refs Turfjs#306

> turf-size rename this turf-resize or something. turf-size is not a good name
> 👍 resize is better. I would also be fine with dropping this entirely. I
> tend to be using different patterns these days around this type of operation
> (explicit distance-based bbox buffers).

* Concave use concaveman, fix versions (Turfjs#361)

* Concave use concaveman, fix versions

* Version

* Fix units consistency. Fixes Turfjs#349 (Turfjs#362)

* turf-concave tests are BS :(

* Port turf-envelope change

* Fix npmignore situation, standardize fixture location (Turfjs#363)

* Documentation improvements

* Documentation improvements

* Documentation fixes

* Update JSTS

* undef

* Remove binaries

* Remove bins in packages

* Fix naive mutations. Fixes Turfjs#116

* Remove turf reclass. Refs Turfjs#306

* Fix indent

* No concaveman (Turfjs#369)

* Revert concaveman switch

* Fix concaveman's fake tests

* Make tests real

* make grid tests real

* Buffer units (Turfjs#371)

* Buffer units

* Make buffer tests real

* Improve docs

* Update deps, improve changelog

* Downgrade title

* Remove lerna from regular deps

* Start publishing section of contributing

* Fix invariant, fix midpoint

* Banner

* v3.0.1

* Update CHANGELOG.md

* clarify changelog on removals

* remove turf-erase in favor of tuf-difference Turfjs#372 (Turfjs#373)

* v3.0.2

* Fix turf-collect package name

* v3.0.3

* Fix turf-along using raw geometry (Turfjs#376)

* coordinate array from raw geometry should pull line.coordinates rather than line.geometry.coordinates

* update test suite to test raw geometry checks

* v3.0.4

* fix meters factor (Turfjs#377)

* v3.0.5

* Update CHANGELOG.md
@easherma
Copy link

resize is better. I would also be fine with dropping this entirely. I tend to be using different patterns these days around this type of operation (explicit distance-based bbox buffers).

@morganherlocker this might be better for a separate thread, but I'm curious if you could elaborate on what you mean by explicit distance-based bbox buffers and why you prefer that method.

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

No branches or pull requests

5 participants