-
-
Notifications
You must be signed in to change notification settings - Fork 337
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
r.to.vect: new flag to re-center centroids #4690
Conversation
A question: Is re-calculating the positions of the centroids computationally intensive (say, what is the expected slow-down)? I ask because it sound that this new flag might even be(come) the default behavior. |
At least for the vector boundary_county from the NC dataset with 4173 centroids, the difference in computional time is not measurable (< 0.05 seconds). The actual vectorization should always take magnitudes more time than any recalculation of the centroids. As a guideline on how much time calculation of centroids takes, you can have a look at the progress of |
There is no simple answer to the increase in computational time: for a large computational region and few areas, the increase will not be noticeable. For a small computational region and many areas, there might be some increase in computational time, but that should not be more than 10% longer, rather 1 - 5% percent longer. |
The blue centroid locations are what I would expect as a centroid. I was surprised to not find any mention of centroids in the docs before (ie, on how the red dots were made). I'm not pronouncing myself on the actual code change and implementation, but this PR received an non-unblocking approval, should it still have another review? Otherwise, I'll leave 5 days up to a week before having it merge. |
I was thinking whether better centroids should be default, but in 99% of cases user won't care about the centroids, so I am fine with this, although I don't have a strong opinion. |
5b4a08d
to
a6fd68d
Compare
@echoix There is no single answer to what is an exact centroid, see v.centerpoint. For the topological GRASS vector model, the only requirement for a centroid is that it must be located inside an area, and this is fulfilled by both the method in As Anna said, in most cases it does not matter where inside an area the centroid is located, thus the slightly slower relocation is optional with the new |
r.to.vect
creates centroids that are often located near boundaries and not in the center of the areas. This PR adds a new-c
flag to re-calculate the position of centroids withVect_get_point_in_area()
as also used by e.g.v.in.ogr
.Red: original centroids calculated by
r.to.vect
, blue: re-centered centroids withr.to.vect -c