Turf: ringEach Proposal

[rowanwins]

So just wanted to continue the discussion from this pull request.

I'd find it quite handy to have a module that looped through interior and exterior rings of polys easily.

turf.ringEach(feature, function (ringCoords, ringIndex, isExterior) { //=ringCoords = [[0,0], [1,1], [1, 0], [0, 0]] //=ringIndex = 0 //=isExterior = true });
@DenisCarriere you proposed splitting the module into interior and exterior versions, I'm just trying to think through whether it would be better to have them together in a single module and how I'd use them....

In an ideal world I'd then love to be able to pass a coord array into one of the other meta functions, eg coordEach without having to first turn into a geojson feature or geometry.

Anyway just starting the discussion rolling in a more trackable place than a vaguely related pull request :)

[DenisCarriere]

you proposed splitting the module into interior and exterior versions

👍 I like your approach better, combining both of them prevents doing a 2nd iteration over the GeoJSON.

In an ideal world I'd then love to be able to pass a coord array into one of the other meta functions, eg coordEach without having to first turn into a geojson feature or geometry.

Agreed, the only thing that we would need to figure out is the depth of the coord Array. Using a Feature or Geometry the depth can be figured out by the Geometry Type, but with an Array it could be a bit trickier (but not impossible).

ringIndex

• [ ] We might want to also include ~ringSubIndex~ multiPartIndex to capture MultiPolygon.

How about this?

turf.ringEach(feature, function (ringCoords, ringIndex, multiPartIndex) {

[stebogit]

@rowanwins @DenisCarriere based on GeoJSON specs the Polygon outer ring has to be the first one. Should we follow and enforce that rule (assuming the input is a valid GeoJSON)? We wouldn't need the isExterior index.

[DenisCarriere]

👍 Agreed, since the OuterRing will always be ringIndex=0

@stebogit How should we handle MultiPolygon Geometries?

Also we should add LineString & MultiLineString as valid inputs (might as well throw in Points as well to support all Geometry types).

[DenisCarriere]

How should we handle MultiPolygon Geometries?

• ringIndex will always be the index for "inner/outer" rings of the Polygons (starting from 0 "exterior ring" and iterate over the inner rings, if MultiPolygon, then there should be multiple ringIndex=0 for each MultiPart)

• ringSubIndex will be the index for the MultiParts, if Feature only has 1 part then ringSubIndex=0 (maybe call it multiPartIndex?)

[rowanwins]

Also we should add LineString & MultiLineString as valid inputs (might as well throw in Points as well to support all Geometry types).

I wasn't thinking about lines or points at all, rings are a polygon thing in my opinion. What would the use case be for lines or points?

How should we handle MultiPolygon Geometries?
Yes this is ought to be a key consideration, obviously this is where you have multiple outer rings so looping becomes useful :)

Maybe rather than talking about ringIndex perhaps we should use the term ringDepth or something like that
turf.ringEach(feature, function (ringCoords, ringDepth, multiPartIndex) { //ringDepth=0 means exterior //ringDepth=1 means interior //multipartIndex })
Although if ringDepth is always 0 or 1 I'd possibly be inclined to go true or false (which take us back almost to my original suggestion :) )

Anyway thanks for your thoughts gents, I think I'll find this module very useful for a lot of polygon related modules and issues around polys with holes

[DenisCarriere]

Maybe rather than talking about ringIndex perhaps we should use the term ringDepth or something like that

I'd still rather use ringIndex, the word index indicates the position that the ring is located in the Geometry.

I wasn't thinking about lines or points at all, rings are a polygon thing in my opinion. What would the use case be for lines or points?

Well we could abstract it even more and call it lineEach & lineReduce which iterates over (Multi)LineString & (Multi)Polygon to retrieve a single LineString.

Using the word ring we now have to define what ring is... if we call it lineEach then it's a bit self-explanatory.

Although if ringDepth is always 0 or 1 I'd possibly be inclined to go true or false (which take us back almost to my original suggestion :) )

Using isExterior might be too specific for a use case, I'd rather use the lineIndex=0 or ringIndex=0 approach.

[rowanwins]

my only concern with a name like lineEach would be that it's potentially confusing if you don't know what you're looking for, looking through a list of modules I wouldn't necessarily expect that to be relevant to polygons...

Happy to bow to pressure on the index naming :)

[DenisCarriere]

Both lineEach or ringEach are both super valid, we can stick with ringEach since flattenEach would solve the use case for doing that same thing for MultiLineString.

• [x] 👍 ringEach & ringReduce
• [x] 👍 Only support for (Multi)Polygon (flattenEach can be used for (Multi)LineString & (Multi)Point have nothing to do with this 😄 )

Next Discussion topic:

~Feature<LineString>~ Feature<Polygon> Callback

Since all the input/output of TurfJS is using Feature/Geometry and not Array<number> (except for Point [number, number]). Shouldn't we return a Feature<Polygon> that way it's easier to apply those new Polygons to all the other TurfJS modules, we're currently doing this for segmentEach using Feature<LineString>.

Since you know 100% that the callback is returning a Feature<Polygon> then you can simply call feature.geometry.coordinates without it ever breaking.

• [x] 👍 Callback should be Feature<Polygon> instead of Array<number>

[DenisCarriere]

Any comments?

turf.ringEach(poly, function (currentRing, ringIndex, multiPartIndex) {
  //=currentRing = Polygon([[[0,0], [1,1], [1, 0], [0, 0]]])
  //=ringIndex = 0
  //=multiPartIndex = 0
});

[rowanwins]

Done!

Now re the callback, I like your idea of it being a feature, my only question would be is it a line or a polygon? I'd argue it's a polygon because the end vertices are purposefully joined... Calling it a line is a tad misleading.

[DenisCarriere]

🤓 Touché! These would be best defined as Polygons since you can't have an open LineString as an inner ring.

• [x] 👍 Feature<Polygon> callback

[rowanwins]

@stebogit is going to wake up in the morning in whatever timezone he is in and everything will be decided without him :) Or perhaps we can just allocate it to him to make the module!

[DenisCarriere]

Haha! 🤣 No this is good, the more we talk about it, it helps define the module, instead of writing code and then changing it 20 commits later.

[stebogit]

Sorry guys, I followed almost all the conversation and I have a couple of suggestions, but I don't have my computer with me! :) I'll join the conversation in a few

[DenisCarriere]

👍 All good, I'll probably end up sleeping on it, so far I don't have anymore comments, looking forward to see what @stebogit has to say.

[stebogit]

So, in my understanding the goal of this module is to loop through the various items represented in the coordinates field of the geometry, being them points, lines or rings. Correct?
With that assumption:

var point, turf.point([5,9]);
turf.geomItemEach(point, function (currentItem, itemIndex, collectionIndex) {
  //=currentItem = point
  //=itemIndex = 0
  //=collectionIndex = undefined
});

var line = turf.lineString([[-0.3,-0.35],[1.45,-0.35],[1.45,1.15],[-0.3,1.15],[-0.3,-0.35]]);
turf.geomItemEach(line, function (currentItem, itemIndex, collectionIndex) {
  //=currentItem = line
  //=itemIndex = 0
  //=collectionIndex = undefined
});

var polygon = turf.poygon([
    [[-0.3,-0.35],[1.45,-0.35],[1.45,1.15],[-0.3,1.15],[-0.3,-0.35]],
    [[0,0],[1,0],[1,1],[0,0]]
]);
turf.geomItemEach(polygon, function (currentItem, itemIndex, collectionIndex) {
  //=currentItem = ring, ring
  //=itemIndex = 0, 1
  //=collectionIndex = undefined
});

//------------

var multiPoint = turf.multiPoint([
   [0,0],[1,1],[2,2]
]);
turf.geomItemEach(multiPoint, function (currentItem, itemIndex, collectionIndex) {
  //=currentItem = point, point, point
  //=itemIndex = 0 | 0 | 0
  //=collectionIndex = 0, 1, 2
});

var multiLine = turf.multiLineString([[
    [[-0.3,-0.35],[1.45,-0.35],[1.45,1.15],[-0.3,1.15],[-0.3,-0.35]],
    [[0,0],[1,0],[1,1],[0,0]] ,
    [[-1.45,-0.95],[-0.90,-2.15],[-0.25,-0.95],[-1.45,-0.95]]
]]);
turf.geomItemEach(multiLine, function (currentItem, itemIndex, collectionIndex) {
  //=currentItem = line | line | line
  //=itemIndex = 0 | 0 | 0
  //=collectionIndex = 0, 1, 2
});

var multiPoly = turf.multiPolygon([[
    [[-0.3,-0.35],[1.45,-0.35],[1.45,1.15],[-0.3,1.15],[-0.3,-0.35]],
    [[0,0],[1,0],[1,1],[0,0]] 
], [
    [[-1.45,-0.95],[-0.90,-2.15],[-0.25,-0.95],[-1.45,-0.95]]
]]);
turf.geomItemEach(multiPoly, function (currentItem, itemIndex, collectionIndex) {
  //=currentItem = ring, ring |  ring
  //=itemIndex = 0, 1 | 0
  //=collectionIndex = 0, 1
});

Does it make sense?

[DenisCarriere]

Well this method would only really apply for polygons, a way to iterate over the inner/outer rings.

At the moment, flattenEach does a lot of the heavy lifting, it would be one level down.

It wouldn't be much logic, but would make iterating over polygons a bit cleaner.

[stebogit]

Which method are you referring to?

[DenisCarriere]

Well I think what @rowanwins wants to get is something like this:

Getting interior/exterior rings of polygons

const {flattenEach} = require('@turf/meta')
const {polygon} = require('@turf/helpers')

var poly = polygon([
    [[-0.3,-0.35],[1.45,-0.35],[1.45,1.15],[-0.3,1.15],[-0.3,-0.35]],
    [[0,0],[1,0],[1,1],[0,0]]
])

flattenEach(poly, (feature, featureIndex, featureSubIndex) => {
    feature.geometry.coordinates.forEach((ringCoords, ringIndex) => {
        const isExterior = ringIndex === 0
        //= ringCoords = Array<number>, Array<number>
        //= ringIndex  = 0, 1
        //= isExterior = true, false
    })
})

After writing this... why are we making a 3 line module which only applies to polygons? lol...

@rowanwins 🤔 Reconsidering this entire module proposal.

Unless we go back to @rowanwins's very first proposal https://github.com/Turfjs/turf/issues/881#issue-247271483 (which makes the most sense now 😝 ).

turf.ringEach(poly, function (ringCoords, ringIndex, isExterior) {
  //=ringCoords = Array<number>, Array<number>
  //=ringIndex = 0, 1
  //=isExterior = true, false
});

[stebogit]

Probably I am missing something here.
As @DenisCarriere said, why would we need a module for only iterating through the rings of a polygon?
You don't even actually need any Turf module:

var poly = polygon([ [[...]], [[...]], [[...]] ]);
poly.geometry.coordinates.forEach((ringCoords, ringIndex) => {
    const isExterior = ringIndex === 0
    //= ringCoords = Array<number>, Array<number>
    //= ringIndex  = 0, 1
    //= isExterior = true, false
})

Maybe we could have a lineEach that applies to (Multi)LineStrings and (Multi)Polygons that allows to iterate through each line in the feature (a ring is by definition a closed LineString):

turf.lineEach(someLineFeature, function (currentLine, lineIndex, multiIndex) {
    //=currentLine
    //=lineIndex (poly's exterior ring => 0)
    //=multiIndex (undefined for LineString)
});

[rowanwins]

I'd argue the same could be said for most of the meta or helper modules, they are simplistic but helpful :)

The reason I'd find it helpful is that when it comes to writing any polygon module it's a pain to repeat those lines when wanting to support polys with holes or multipolys. It's one less chunk of code I have to remember or think about writing which gives me more headspace to support those use cases.

I think @stebogit snippet above is probably the most helpful in achieving what I want so perhaps I'll give that a whirl and report back.

[DenisCarriere]

👍 @stebogit I like lineEach, seems a bit more flexible.

Great back and forth discussions :)

[stebogit]

@rowanwins shall we close this?

[rowanwins]

Consider it closed - thanks @stebogit !