randomLineString does not take max_rotation option into account
Nobody has claimed this yet.
- Dominant language
- TypeScript
- Stars
- 10.5k
- Forks
- 1k
- Avg merge
- 1h 11m
- Merged PRs (30d)
- 4
Description
It seems to be related to the fact that the previous angle variable is actually a local variable of the inner loop over the vertices so that it is incorrectly reset at each vertex. Here is a suggested modification of the code, which also appears to be more simple:
for (var i = 0; i < count; i++) {
var startingPoint = turf.randomPosition(bbox);
var vertices = [startingPoint];
var priorAngle = Math.random() * 2 * Math.PI;
for (var j = 0; j < num_vertices - 1; j++) {
var angle = priorAngle + (Math.random() - 0.5) * max_rotation * 2;
priorAngle = angle;
var distance = Math.random() * max_length;
vertices.push([
vertices[j][0] + distance * Math.cos(angle),
vertices[j][1] + distance * Math.sin(angle),
]);
}
features.push(turf.lineString(vertices));
}
I would be pleased to try to create a PR if you think this fix is correct, althought it might take some time to me for a first contribution to study how to write a relevent test, update the build, etc.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in packages/turf-random/index.ts around line 237 and inspect how randomLineString handles max_rotation and the prior angle. Add a regression test covering max_rotation, then run the relevant package tests and update the build if required; done means the option affects successive vertices as described.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- data
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 48/100