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.convex ignores given properties #2762

Open
3 of 4 tasks
kareemjeiroudi opened this issue Dec 4, 2024 · 2 comments · May be fixed by #2765
Open
3 of 4 tasks

turf.convex ignores given properties #2762

kareemjeiroudi opened this issue Dec 4, 2024 · 2 comments · May be fixed by #2765

Comments

@kareemjeiroudi
Copy link

kareemjeiroudi commented Dec 4, 2024

Background

In my sample code below, the function combines a list of adjacent polygons into one. Every polygon shows its cadastral information such as cadastral community number, as well as plot number. When combining these polygons into a single convex, I want to combine the numbers into list, that is populated in the convex additional properties. The function seems to ignore the given properties though. Please observe the source code of the convex function.

I would like to open a pull request to fix this problem. I just need approval or confirmation from someone that they were able to reproduce the problem.

convex-polygons

  • The version of Turf you are using, and any other relevant versions.
    6.5.0
  • GeoJSON data as a gist file or geojson.io (filename extension must be .geojson).
    Issue is rather simple, and doesn't need test data. Should be reproducible with any GeoJSON.
  • Snippet of source code or for complex examples use jsfiddle.
type AdditionalProperties = {
  cadastralCommunity: string[];
  plots: string[];
};

type TurfFeature = turf.Feature<turf.Polygon | turf.MultiPolygon, AdditionalProperties>;

function combinePolygons(polygons: TurfFeature[]) {
  const combinedPolygons = polygons.reduce<TurfFeature>((previousPolygon, currentPolygon) => {
    if (previousPolygon == null) {
      return currentPolygon;
    }

    const reducedPlotNumbers = [...previousPolygon.properties.plots, ...currentPolygon.properties.plots];
    const reducedCadastralCommunities = [
      ...previousPolygon.properties.cadastralCommunity,
      ...currentPolygon.properties.cadastralCommunity,
    ];

    return turf.union<AdditionalProperties>(previousPolygon, currentPolygon, {
      properties: {
        plots: reducedPlotNumbers,
        cadastralCommunity: reducedCadastralCommunities,
      },
    });
  }, null);

  if (combinedPolygons.geometry.type === 'Polygon') {
    return combinedPolygons;
  }

  const { properties } = combinedPolygons;
  const combinedConvex = turf.convex<AdditionalProperties>(combinedPolygons.geometry, {
    // there's a bug in the convex function where the properties are not populated https://github.com/Turfjs/turf/blob/master/packages/turf-convex/index.ts
     properties: combinedPolygons.properties
  });

  if (combinedConvex) {
    // this should not be necessary
    combinedConvex.properties = properties;
  }

  return combinedConvex;
}
  • Verify this issue hasn't already been reported, or resolved in the latest alpha pre-release.
    I couldn't find an issue matching this reported issue.
@smallsaucepan
Copy link
Member

Yeah. Wow. That is definitely a bug. Thanks for reporting this @kareemjeiroudi.

Please go ahead and submit a PR 👍

kareemjeiroudi added a commit to kareemjeiroudi/turf that referenced this issue Dec 10, 2024
kareemjeiroudi added a commit to kareemjeiroudi/turf that referenced this issue Dec 10, 2024
@kareemjeiroudi kareemjeiroudi linked a pull request Dec 10, 2024 that will close this issue
2 tasks
@kareemjeiroudi
Copy link
Author

Just opened a PR to resolve this issue. Please let me know if the PR needs improvement or if missing additional information.
#2765

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

Successfully merging a pull request may close this issue.

2 participants