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

Patch for point-grid bug #1887

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Patch for point-grid bug #1887

wants to merge 2 commits into from

Conversation

entagir
Copy link

@entagir entagir commented Apr 23, 2020

Correction of xFraction variable calculation.

I propose to calculate the distance in latitude from the edge of the box to the center and multiply by 2, because with a large box (more than half the length of the latitude), the distance is not calculated correctly (the shortest distance from edge to edge will envelope the Earth from the other side and will not be the length of the box).

I also propose to calculate the distance in latitude, located in the center of the height of the box, because Now it is calculated on the lower edge (south), which gives a very small number of points for large boxes (at a height from the equator to the pole), because at the northern and southern latitudes their length decreases with distance from the equator. Also, with the same small boxes (one in the northern hemisphere, the other in the southern), the difference in result will be visible. In this regard, I think it will be fair to consider the latitude located in the center of the box.

And you also need to revise the tests for this module.

Thank.

PS: I attach the results and testing code to the tests of the module turf-point-grid. For comparison.
turf-point-grid-test.zip

Resolves #1886

Correction of xFraction variable calculation.

I propose to calculate the distance in latitude from the edge of the box to the center and multiply by 2, because with a large box (more than half the length of the latitude), the distance is not calculated correctly (the shortest distance from edge to edge will envelope the Earth from the other side and will not be the length of the box).

I also propose to calculate the distance in latitude, located in the center of the height of the box, because Now it is calculated on the lower edge (south), which gives a very small number of points for large boxes (at a height from the equator to the pole), because at the northern and southern latitudes their length decreases with distance from the equator. Also, with the same small boxes (one in the northern hemisphere, the other in the southern), the difference in result will be visible. In this regard, I think it will be fair to consider the latitude located in the center of the box.

And you also need to revise the tests for this module.

Thank.
@entagir entagir mentioned this pull request Apr 23, 2020
@entagir entagir changed the title Patch for point-grid bag Patch for point-grid bug Apr 23, 2020
@JamesLMilner
Copy link
Collaborator

@rowanwins is this PR still relevant after your changes here #2106 ? If not we can close perhaps?

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

Successfully merging this pull request may close these issues.

point-grid bug
2 participants