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

Standardize incoming geometry (handle FeatureCollections) #5

Open
mapsam opened this issue Mar 22, 2015 · 10 comments
Open

Standardize incoming geometry (handle FeatureCollections) #5

mapsam opened this issue Mar 22, 2015 · 10 comments

Comments

@mapsam
Copy link
Member

mapsam commented Mar 22, 2015

Right now feature collections are a bit iffy. The ops.geom.union() function, for example, tests to see if a passed object has object.features, which means it's a feature collection and then uses the first feature object.features[0] to do the operation. This doesn't seem very good!

@mapsam mapsam mentioned this issue Mar 22, 2015
Merged
@mapsam
Copy link
Member Author

mapsam commented Mar 24, 2015

I wonder if we can ensure that ALL features are converted to feature collections on upload and handle them this way? As long as everything is formatted the same, then we don't have to build if checks on them.

@mapsam
Copy link
Member Author

mapsam commented Mar 24, 2015

Using turf.featureCollection(feature) should make this pretty 🍰

@morganherlocker
Copy link

Check out geojson-normalize and geojson-flatten. Running everything through both of these tends to smooth things out with highly variable input.

geojson = normalize(flatten(geojson))

@mapsam
Copy link
Member Author

mapsam commented Apr 2, 2015

Oh! Just to be clear, are you saying use the npm packages for these? geojson-normalize & geojson-flatten

Those seem super helpful. Presumably the best way to go about doing this would be getting #8 moving forward so accessing these and future modules will be a breeze.

@mapsam
Copy link
Member Author

mapsam commented Apr 14, 2015

@alukach it looks like this will be a good next issue to resolve before we add all of the Turf operations since #30 re-introduced some errors dealing with different geom types. How do you propose we incorporate other modules if we're not using browserify any longer? Maybe we should re-introduce browserify for things like these?

@mapsam mapsam changed the title handle FeatureCollections Standardize incoming geometry (handle FeatureCollections) Apr 14, 2015
@mapsam mapsam added the turf label Apr 15, 2015
@mapsam
Copy link
Member Author

mapsam commented Apr 21, 2015

Does it make sense to have every layer inside a single feature collection? That would allow us to simplify the selection system into specific geometries.

I was hesitant about this approach because it makes handling features with multiple polygons difficult, but now that I look into the geojson spec a bit more I realize that this is a non-issue since geometry is it's own object class within a feature and therefore contains multiPolygons as if we had a feature collection.

Therefore, does the entire layerlist become a representation of EACH feature within the global featureCollection? I'd like to see layerlist respond to an updated map featureCollection, rather than update the map from the layerlist.

@mapsam
Copy link
Member Author

mapsam commented Apr 21, 2015

Hm, now that I think about it (again) I'm realizing that the above would require ALL leaflet layers to be mushed into a single layer, which doesn't seem good. I wonder if we can convert all featureCollections into single features with multiGeometry attributes instead.

alukach added a commit that referenced this issue Apr 28, 2015
adding function to remove single feature FeatureCollections, #5
@mapsam mapsam modified the milestone: Pre-alpha (Spring Fling) Apr 28, 2015
@nbolten
Copy link

nbolten commented May 14, 2015

geojson-normalize and geojson-flatten solve the issue of non-uniformity in the input (everything becomes a featureCollection). I think that would solve the immediate issue of knowing where the geometries reside in a given layer.

I ran into issues making some new Turf.js operations work due to mismatches between the layer's data type and the operation's expected input data type. e.g., union currently needs exactly two Polygon inputs from separate layers - if the Polygons are in e.g. FeatureCollections or a single layer, a cryptic Javascript error pops up on the console. In addition, it's non-obvious to the end user that exactly two Polygons are expected. Should I open a separate issue for matching up layer data types (geojson) with Turf.js operations?

(Also thanks for the CUGOS Spring Fling, it was super fun!)

@mapsam
Copy link
Member Author

mapsam commented May 14, 2015

@nbolten thanks for the insight! That's exactly what's going on - and should be solved sooner than later. It's going to be difficult to make sure everything matches what turf expects, but if we can at least make sure every feature coming in is converted to a featureCollection that should give us a standardized stepping stone.

@alukach was a bit hesitant to muddle geometry that a user uploads (i.e. convert to a different type), which is understandable. How do you think we should move forward?

@mapsam mapsam modified the milestones: Pre-alpha (Spring Fling), Alpha Sep 8, 2015
@mapsam
Copy link
Member Author

mapsam commented Sep 17, 2015

Final thoughts on this after my incoherent mental back-and-forth:

Rather than relying on the incoming geometry that turf requires, I'd like to give ourselves one extra step of granularity with the geo functions. Right now we run all turf functions as-is, no questions asked by referencing the operation by name from a list of operations we've defined ourselves, like the following:

var ops = {
  buffer: {
    parameters: ['important', 'parameters']
  },
  within: {
    parameters: [1, 'another parameter']
  }
};

// assumes parameters includes the geojson objects as well
function executeTurfOperation(operation) {
  var result = turf[operation].apply(null, ops[operation].parameters);
  return result;
}

var myNewGeoJSON = executeTurfOperation('buffer');

This makes the executeTurfOperations function inflexible, and makes us dependent on the geometry types that Turf requires, which can be confusing in a GIS in examples such as turf.within() requiring two feature collections when passing a single Feature<Polygon> could also be used to see if points exist within the extent. This is confusing to the user, and makes the geo operations brittle, and nearly useless unless a user clearly understands GeoJSON ((which is not always the case)[https://github.com//issues/10#issuecomment-99639758]).

Relying on sending our own commits/updates to the Turf repos (which admittedly sounds awesome) and waiting on updates, I think we should give ourselves an opportunity to write our own spatial functions and execute them accordingly, while still harnessing the power that turf gives us (see the turf-within code and how it uses turf.inside()). I propose the following update to the code above:

var ops = {
  buffer: {
    parameters: ['important', 'parameters'],
    execute: function(parameters) {
      // standard usage of turf.buffer()
      var geojson = turf.buffer.apply(null, parameters);
      return geojson;
    }
  },
  within: {
    parameters: [1, 'another parameter'],
    execute: function(parameters) {
      // custom implementation of turf.within()
      var params = parameters;

      // if the feature is a FeatureCollection, just go ahead and use it
      // otherwise if it is a Feature<Polygon> let's convert it to a FeatureCollection
      // so we can still use it
      if (parameters[0].type !== 'FeatureCollection') {
        params = someCustomFunctionThatReturnsNewParameters();
      }

      var geojson = turf.within.apply(null, params);
      return geojson;
    }
  }
};

function executeTurfOperation(operation) {
  // assumes parameters includes the geojson objects as well from a submitted form
  var result = ops[operation].execute(ops[operation].parameters);
  return result;
}

var myNewGeoJSON = executeTurfOperation('buffer');

This allows us to continue using functions as-is with Turf, but gives us a little breathing room during their execution in order to allow for more geometry types.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants