dropchop

dropchop copied to clipboard

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!

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.

Using turf.featureCollection(feature) should make this pretty :cake:

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))

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.

@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?

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.

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.

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!)

@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?

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/cugos/dropchop/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.