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

transformScale doesn't always create similar shapes #895

Closed
DingoEatingFuzz opened this issue Aug 10, 2017 · 12 comments
Closed

transformScale doesn't always create similar shapes #895

DingoEatingFuzz opened this issue Aug 10, 2017 · 12 comments
Assignees
Labels
Milestone

Comments

@DingoEatingFuzz
Copy link

Triangles become scalene and hexagons go concave.

I discovered this while working in Glitch. Here's the project link: https://glitch.com/edit/#!/nosy-flare?path=map.js:96:8

image

@rowanwins
Copy link
Member

Thanks for the report @DingoEatingFuzz

This is one for you @stebogit :)

@rowanwins rowanwins added the bug label Aug 11, 2017
@DenisCarriere
Copy link
Member

Very interesting... 🤔

@stebogit
Copy link
Collaborator

@DingoEatingFuzz the issue has something to do with the mutate parameter, I'll dig more into it in the weekend.
For now you can create a new scaled grid:

var scaledGrid = turf.featureCollection([]);
scaledGrid.features = processedGrid.features.map(f => {
  var factors = f.properties.list.map(ms => (ms - minDate) / dateDiff);
  var avgFactor = factors.reduce((sum, n) => sum + n, 0) / factors.length;
  return turf.transformScale(f, avgFactor * (maxFactor - minFactor) + minFactor);
})

map.getSource('grid').setData(scaledGrid);

screen shot 2017-08-11 at 1 37 07 am

@DenisCarriere
Copy link
Member

👍 @stebogit I'll look into this, I might know where the issue is on this one. Also we can add @turf/clone to the mix.

@DenisCarriere
Copy link
Member

Strange one... 🤔

I've been able to get the same "expected" results as @stebogit using a slightly different approach.

processedGrid.features = processedGrid.features.map(feature => {
  var factors = feature.properties.list.map(ms => (ms - minDate) / dateDiff);
  var avgFactor = factors.reduce((sum, n) => sum + n, 0) / factors.length;
  var factor = avgFactor * (maxFactor - minFactor) + minFactor;
  return turf.transformScale(feature, factor, 'centroid')
})

@stebogit This is the type of "strange" unexpected mutations that @morganherlocker has been advocating, hence why all of the early TurfJS modules don't have Mutate as param since it's hard to control the expected outputs.

I initially added the mutate param for testing purposes.

@stebogit
Copy link
Collaborator

@DenisCarriere this is really interesting, I thought it was not possible to map an array to "itself"

processedGrid.features = processedGrid.features.map(feature => {

I guess JS creates a temporary copy of the array then. Cool! 👍

@DenisCarriere
Copy link
Member

Yep! map stores a temp array, that's why it's a bit slower than doing featureEach or for loops.

@DenisCarriere
Copy link
Member

I'm still 🤔 why this issue happening... don't know why this would cause this.

@DenisCarriere
Copy link
Member

Maybe clone the origin point as well?

@DenisCarriere
Copy link
Member

@stebogit Ok I've been able to make a test case for it.

image

DenisCarriere added a commit that referenced this issue Aug 11, 2017
@DenisCarriere
Copy link
Member

DenisCarriere commented Aug 11, 2017

OOOOH I got it!
Since we are mutating the Polygon geometry, the first coordinate gets mutated and whenever it gets to the end, it uses the previously mutated coordinate instead of the original coordinate.

I think I've got a fix for this.

The scaling happens twice on the first & last coordinate position (for some reason...)

@DenisCarriere DenisCarriere self-assigned this Aug 11, 2017
@DenisCarriere DenisCarriere added this to the 4.7.0 milestone Aug 11, 2017
stebogit added a commit that referenced this issue Aug 13, 2017
cloned last vertex in turf-hex-grid;
reverted changes in transform-scale;
fixed JDoc types;
DenisCarriere referenced this issue Aug 14, 2017
Add FeatureCollection mutate tests (not failing)
@amirnissim
Copy link

I believe there's still an issue when passing mutate: true : #2725

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

No branches or pull requests

5 participants