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.reclass.area: Add flag to optionally output NULL raster if no areas are left after reclass #1728

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions scripts/r.reclass.area/r.reclass.area.html
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ <h2>DESCRIPTION</h2>
If the <b>-c</b> flag is used, <em>r.reclass.area</em> will skip the
creation of a clumped raster and assume that the input raster is
already clumped.
<p>
If the <b>-n</b> flag is used, <em>r.reclass.area</em> will output a raster
with only NULL values if no areas are left after applying the reclass
<b>method</b> (instead of throwing an error).

<h2>EXAMPLES</h2>

Expand Down
49 changes: 35 additions & 14 deletions scripts/r.reclass.area/r.reclass.area.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,11 @@
# % description: Clumps including diagonal neighbors
# %end

# %flag
# % key: n
# % description: Create NULL map if no areas are left after reclass method
# %end

import sys
import os
import atexit
Expand All @@ -81,7 +86,7 @@
TMPRAST = []


def reclass(inf, outf, lim, clump, diag, les):
def reclass(inf, outf, lim, clump, diag, les, nodata=False):
infile = inf
outfile = outf
lesser = les
Expand Down Expand Up @@ -163,18 +168,29 @@ def reclass(inf, outf, lim, clump, diag, les):
p1.wait()
p2.stdin.close()
p2.wait()
expression = "$outfile = $recfile"
mapcalc_kwargs = {"outfile": outfile, "recfile": recfile}
if p2.returncode != 0:
if lesser:
grass.fatal(
_("No areas of size less than or equal to %f " "hectares found.")
% limit
op_str = "less"
else:
op_str = "greater"
Comment on lines +175 to +177
Copy link
Member

Choose a reason for hiding this comment

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

The original code is right. Sentences cannot be build from parts because that makes translations impossible. If really needed, you could create two sentences and then put them together using another, possibly translatable, string.

if nodata is True:
grass.warning(
Copy link
Member

Choose a reason for hiding this comment

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

You are probably in the best position to decide that, but if user requests NULL raster in those cases, is warning needed? Isn't that a verbose message instead? In other words, the new flag seems to make the the "no areas" state to be a standard state with appropriate output, so why a warning?

_(
"No areas of size %s than or equal to %f "
"hectares found. Creating NULL raster..."
)
% (op_str, limit)
)
expression = "$outfile = null()"
del mapcalc_kwargs["recfile"]
Comment on lines +186 to +187
Copy link
Member

Choose a reason for hiding this comment

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

Maybe having two calls of mapcalc would result in a simpler code.

else:
grass.fatal(
_("No areas of size greater than or equal to %f " "hectares found.")
% limit
_("No areas of size %s than or equal to %f " "hectares found.")
% (op_str, limit)
Copy link
Member

Choose a reason for hiding this comment

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

To improve the code overall, please use .format() when rewriting the messages.

)
grass.mapcalc("$outfile = $recfile", outfile=outfile, recfile=recfile)
grass.mapcalc(expression, **mapcalc_kwargs)


def rmarea(infile, outfile, thresh, coef):
Expand Down Expand Up @@ -209,6 +225,7 @@ def main():
method = options["method"]
clumped = flags["c"]
diagonal = flags["d"]
nodata = flags["n"]

# check for unsupported locations
in_proj = grass.parse_command("g.proj", flags="g")
Expand All @@ -226,7 +243,9 @@ def main():
grass.fatal(_("Raster map <%s> not found") % infile)

if method == "reclass":
reclass(infile, outfile, limit, clumped, diagonal, mode == "lesser")
reclass(
infile, outfile, limit, clumped, diagonal, mode == "lesser", nodata=nodata
)
elif method == "rmarea":
rmarea(infile, outfile, limit, in_proj["meters"])

Expand All @@ -238,13 +257,15 @@ def cleanup():
TMPRAST.reverse() # reclassed map first
for mapp in TMPRAST:
if method == "rmarea":
grass.run_command(
"g.remove", flags="f", type="vector", name=mapp, quiet=True
)
if grass.find_file(name=mapp, element="vector")["file"]:
Copy link
Member

Choose a reason for hiding this comment

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

Please comment on why you are making this change in the PR description.

grass.run_command(
"g.remove", flags="f", type="vector", name=mapp, quiet=True
)
else:
grass.run_command(
"g.remove", flags="f", type="raster", name=mapp, quiet=True
)
if grass.find_file(name=mapp, element="raster")["file"]:
grass.run_command(
"g.remove", flags="f", type="raster", name=mapp, quiet=True
)


if __name__ == "__main__":
Expand Down
Loading