-
Notifications
You must be signed in to change notification settings - Fork 954
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
isobands fails on flat data #1797
Comments
You're right, the output in your case should be a |
Do you have advice for how you would like to see this fixed? Are you open to merging PRs? (I assume to the v7 branch, but if that is not where you want them say so) How much of a rewrite/cleanup is exemptible? (for example the I have spent the past week trying to dig in. It looks like the boundary edges are added in the loop in BandGrid2AreaPaths, but that is only triggered when an internal edge hits the boundary. I think we can separate the core of that loop into a function, and then call the function on (0, 0) if it should be in the band. But I am very open to advice. |
@Eh2406 I originally used MarchingSquares.js importing the library via mpn, then, on an unsuccessful attempt to convert Turf to TypeScript, the library was included as local dependency. |
You are reading our minds! I was just going to comment that it looked like you had copied the code from there and that the npm version does not have this bug. Given that the v7 branch is giving up on TypeScript, do you want a PR moving back to using the npm version? |
👍 |
…s library from NPM. This commit supercedes PR Turfjs#1801 which is a few years old now. Resolves Turfjs#1797 based on a test case mentioned in that issue (added to unit tests in isobands). Slight changes to resulting coordinates in most fixtures, though all fixtures verified visually using geojson.io
…s library from NPM. This commit supercedes PR Turfjs#1801 which is a few years old now. Resolves Turfjs#1797 based on a test case mentioned in that issue (added to unit tests in isobands). Slight changes to resulting coordinates in most fixtures, though all fixtures verified visually using geojson.io
…s library from NPM. This commit supercedes PR Turfjs#1801 which is a few years old now. Resolves Turfjs#1797 based on a test case mentioned in that issue (added to unit tests in isobands). Slight changes to resulting coordinates in most fixtures, though all fixtures verified visually using geojson.io
…st break" bug (#2527) * Migrating isobands and isolines to use the third party marchingsquares library from NPM. This commit supercedes PR #1801 which is a few years old now. Resolves #1797 based on a test case mentioned in that issue (added to unit tests in isobands). Slight changes to resulting coordinates in most fixtures, though all fixtures verified visually using geojson.io * Fixing turf-isolines to no longer ignore the first break passed. Where it looked like the unit tests have had a 0 break added as a work around e.g. 5 breaks but only 4 break properties, have removed the presumed workaround and reviewed test fixture output. Resolves #2129. * Rolling back cloning workaround from commit d7c5322. This appears to have been unnecessary, possibly already having been fixed by adding linearRing: false to the isoContours call.
We are hitting this in a map @superchad and I are developing. When all the points in the grid fall in the same band then
isobands
returns no geometry instead of the bbox of the grid. If one point on the edge has a different band than the hole area is filled in correctly. Instead of a geojson I have a failing test v7...Eh2406:v7The text was updated successfully, but these errors were encountered: