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

r.horizon: adjust bufferzone to multiples of resolution #3384

Merged
merged 5 commits into from
Feb 14, 2024

Conversation

petrasovaa
Copy link
Contributor

Fixes #3266. Now it adjust the bufferzone, e.g. for 10m resolution raster, bufferzone 109 gets adjusted to 100.

Just to see the what effect this had, this is a difference between a horizon raster where bufferzone is 100 (even multiples of resolution 10) and 103. Now there is no difference (see the test).
Selection_276

@petrasovaa petrasovaa added this to the 8.4.0 milestone Jan 29, 2024
@github-actions github-actions bot added raster Related to raster data processing Python Related code is in Python C Related code is in C module labels Jan 29, 2024
@petrasovaa petrasovaa self-assigned this Jan 29, 2024
@petrasovaa petrasovaa added the bug Something isn't working label Jan 30, 2024
raster/r.horizon/main.c Show resolved Hide resolved
raster/r.horizon/main.c Show resolved Hide resolved
@echoix echoix enabled auto-merge (squash) February 14, 2024 01:31
@echoix echoix disabled auto-merge February 14, 2024 01:32
@echoix echoix enabled auto-merge (squash) February 14, 2024 01:33
@echoix
Copy link
Member

echoix commented Feb 14, 2024

@petrasovaa I let you resolve the last comment, and it will merge automatically ;) Then it will be time to resolve the conflicts for the other r.horizon PRs

@echoix echoix merged commit 38e0b3d into OSGeo:main Feb 14, 2024
25 checks passed
petrasovaa added a commit that referenced this pull request Feb 14, 2024
* r.horizon: adjust bufferzone to multiples of resolution

* fix test

* restrict input bufferzone to positive values

* add tests for negative bufferzone
@petrasovaa
Copy link
Contributor Author

Backport breaks tests, because of a bug in pygrass, either I'll remove the test from 8.3 or #2817 needs to be backported. @wenzeslaus can we backport it?

@wenzeslaus
Copy link
Member

I recommend removing the test. It is the least invasive change for the upcoming micro release of v8.3 which is close. While the improvement in the message is significant, the fix is may cause trouble if someone wrote their code around it without reading the doc. Next minor, i.e., 8.4, which is relatively close, is a better place for it.

jadenabrams100 pushed a commit to ncsu-csc472-spring2024/grass-CI-playground that referenced this pull request Feb 21, 2024
* r.horizon: adjust bufferzone to multiples of resolution

* fix test

* restrict input bufferzone to positive values

* add tests for negative bufferzone
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working C Related code is in C module Python Related code is in Python raster Related to raster data processing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] r.horizon creates strange offset if bufferzone is not multiple of raster resolution
3 participants