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

[gdal_contour] Do not include min/max when polygonize #11673

Merged
merged 4 commits into from
Jan 16, 2025

Conversation

elpaso
Copy link
Collaborator

@elpaso elpaso commented Jan 16, 2025

Fixes #11564 by not including raster min/max when polygonize and fixed levels are specified.

MIN and MAX literals can be used to specify raster min/max values, e.g.:

-fl MIN 10 20 30 MAX

@elpaso elpaso added the funded through GSP Work funded through the GDAL Sponsorship Program label Jan 16, 2025
@elpaso elpaso force-pushed the bugfix-gh11564-contour-fixed-levels branch from 15b7e42 to 0d8138f Compare January 16, 2025 10:45
Fixes OSGeo#11564 by not including raster min/max when polygonize and
fixed levels are specified.

MIN and MAX literals can be used to specify raster min/max values, e.g.:

-fl MIN 10 20 30 MAX
@elpaso elpaso force-pushed the bugfix-gh11564-contour-fixed-levels branch from 0d8138f to 26d9230 Compare January 16, 2025 11:10

This would create 10-meter polygonal contours from the DEM data in :file:`test.asc` and produce a GeoJSON output
with the contour min and max elevations in the ``min`` and ``max`` attributes, the values of these fields will
be: (4.0, 10.0), (10, 15.0), (15, 20.0) and (20, 36.0).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shoud it be (10, 15.0), (15, 20.0) and (20, 30.0)?

I think I understand what happens, but not how to describe it so that a newcomer understands. Perhaps something like
"Creates contours at regular 10 meter intervals and adds extra contour for a fixed 15 m level. Finally turns areas between the contours into polygons."

Copy link
Collaborator

@jratike80 jratike80 Jan 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For those values, shouldn't user define -i 10 -fl MIN 15 MAX -p? Otherwise it would not match with the title. Or do regular intervals start/stop at min/max automatically? It does make sense, most users want to get the polygon between the sea and 10 m level automatically, but it is not clear from the documentation.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a bit different approach would have been easier to explain. First improve documentation of -p

"Generate contour polygons rather than contour lines. The min and max values of the raster will be used as the lower and upper limit for the data range of the first and last polygon, respectively.

Then introduce exlude_min and exclude_max for preventing the default behavior so that it would have similar effect on all -i and -fl combinations, when used together with -p, and no effect without -p.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For those values, shouldn't user define -i 10 -fl MIN 15 MAX -p? Otherwise it would not match with the title. Or do regular intervals start/stop at min/max automatically? It does make sense, most users want to get the polygon between the sea and 10 m level automatically, but it is not clear from the documentation.

yes, regular intervals get min/max automatically. I didn't change that.

@elpaso elpaso force-pushed the bugfix-gh11564-contour-fixed-levels branch from e8da296 to d5a1040 Compare January 16, 2025 12:07
The minimum and maximum values from the raster are not automatically added to
the fixed levels list but the special values `MIN`` and `MAX`` (case insensitive)
can be used to include them.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... but when option -i is used together with -p, the minimum and maximum values from the raster ARE automatically added, and there is no way to get rid of them.

I know this is not a common use case, but there can be lot of writing to the -fl if the DEM has a large height range.

This would create 10-meter polygonal contours from the DEM data in :file:`test.asc`
and produce a GeoJSON output with the contour min and max elevations in the ``min``
and ``max`` attributes, not including the minimum and maximum values from the raster.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not understand. If there is only -i, doesn't it mean, that min/max are used just like before the change?
Or maybe I just need another cup of coffee.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You a re right: I removed the not.

@rouault rouault added this to the 3.11.0 milestone Jan 16, 2025
@rouault rouault merged commit 621a86a into OSGeo:master Jan 16, 2025
36 of 37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
funded through GSP Work funded through the GDAL Sponsorship Program
Projects
None yet
Development

Successfully merging this pull request may close these issues.

gdal_contour generates polygon for value not specified in fixed levels
3 participants