Skip to content

r.fillnulls: avoid locale-dependent stderr parsing #6930

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.

Sign up for GitHub

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

Open
Abhi-d-gr8 wants to merge 12 commits into
base: main
Choose a base branch
from
Open

r.fillnulls: avoid locale-dependent stderr parsing #6930

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
0881726
r.fillnulls: avoid parsing translated stderr for no-NULL case
Jan 22, 2026
6b762fc
tests: add r.fillnulls no-NULL input case
Jan 23, 2026
8b1e6ea
Update scripts/r.fillnulls/testsuite/test_r_fillnulls.py
Abhi-d-gr8 Jan 25, 2026
c1cd9c5
style: fix trailing whitespace in r.fillnulls tests
Jan 27, 2026
701af0a
r.fillnulls: handle no-NULL rasters without error
Abhi-d-gr8 Jan 28, 2026
2e77e3e
r.fillnulls: handle no-NULL rasters via errno from r.resamp.bspline
Abhi-d-gr8 Jan 30, 2026
a46f8d4
r.resamp.bspline: set errno for no-NULL case with -n
Abhi-d-gr8 Jan 30, 2026
64bc53e
Apply suggestions from code review
nilason Jan 30, 2026
50e2ec9
Update raster/r.resamp.bspline/main.c
nilason Jan 30, 2026
818adaa
Update scripts/r.fillnulls/testsuite/test_r_fillnulls.py
nilason Jan 30, 2026
27d36a1
Merge branch 'main' into fix-1495-r-fillnulls-locale
nilason Jan 30, 2026
660928a
tests: fix precision handling in r.fillnulls no-NULL test
Abhi-d-gr8 Jan 31, 2026
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
2 changes: 2 additions & 0 deletions raster/r.resamp.bspline/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <sys/stat.h>
#include <fcntl.h>
#include <math.h>
#include <errno.h>
#include "bspline.h"

#define SEGSIZE 64
Expand Down Expand Up @@ -456,6 +457,7 @@ int main(int argc, char *argv[])
Rast_close(maskfd);
}
if (null_flag->answer && null_count == 0 && !mask_opt->answer) {
errno = EINVAL;
G_fatal_error(_("No NULL cells found in input raster."));
}
}
Expand Down
76 changes: 23 additions & 53 deletions scripts/r.fillnulls/r.fillnulls.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@

import os
import atexit
import subprocess
import errno

import grass.script as gs
from grass.exceptions import CalledModuleError
Expand Down Expand Up @@ -556,26 +556,19 @@ def main():
)
)

# check if method is different from rst to use r.resamp.bspline
if method != "rst":
gs.message(_("Using %s bspline interpolation") % method)

# clone current region
gs.use_temp_region()
gs.run_command("g.region", align=input)

reg = gs.region()
# launch r.resamp.bspline
tmp_rmaps.append(prefix + "filled")
# If there are no NULL cells, r.resamp.bslpine call
# will end with an error although for our needs it's fine
# Only problem - this state must be read from stderr
new_env = dict(os.environ)
new_env["LC_ALL"] = "C"
if usermask:
try:

try:
if usermask:
with gs.MaskManager():
p = gs.core.start_command(
gs.run_command(
"r.resamp.bspline",
input=input,
mask=usermask,
Expand All @@ -586,30 +579,9 @@ def main():
lambda_=lambda_,
memory=memory,
flags="n",
stderr=subprocess.PIPE,
env=new_env,
)
stderr = gs.decode(p.communicate()[1])
if "No NULL cells found" in stderr:
gs.run_command(
"g.copy", raster="%s,%sfilled" % (input, prefix), overwrite=True
)
p.returncode = 0
gs.warning(
_(
"Input map <%s> has no holes. Copying to output without "
"modification."
)
% (input,)
)
except CalledModuleError:
gs.fatal(
_("Failure during bspline interpolation. Error message: %s")
% stderr
)
else:
try:
p = gs.core.start_command(
else:
gs.run_command(
"r.resamp.bspline",
input=input,
output=prefix + "filled",
Expand All @@ -619,27 +591,25 @@ def main():
lambda_=lambda_,
memory=memory,
flags="n",
stderr=subprocess.PIPE,
env=new_env,
)
stderr = gs.decode(p.communicate()[1])
if "No NULL cells found" in stderr:
gs.run_command(
"g.copy", raster="%s,%sfilled" % (input, prefix), overwrite=True
)
p.returncode = 0
gs.warning(
_(
"Input map <%s> has no holes. Copying to output without "
"modification."
)
% (input,)

except CalledModuleError as e:
# r.resamp.bspline sets exit status to errno (EINVAL) for "no NULL cells"
if e.returncode == errno.EINVAL:
gs.warning(
_(
"Input map <%s> has no holes. Copying to output without "
"modification."
)
except CalledModuleError:
gs.fatal(
_("Failure during bspline interpolation. Error message: %s")
% stderr
% (input,)
)
gs.run_command(
"g.copy",
raster="%s,%sfilled" % (input, prefix),
overwrite=True,
)
else:
raise

# set region to original extents, align to input
gs.run_command(
Expand Down
20 changes: 20 additions & 0 deletions scripts/r.fillnulls/testsuite/test_r_fillnulls.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,26 @@ def test_basic(self):
precision=1e-6,
)

def test_no_nulls(self):
"""Test r.fillnulls when input raster has no NULL cells"""
# elevation has no NULL cells by default
module = SimpleModule(
self.module,
input=self.mapName,
output=self.mapComplete,
overwrite=True,
)

# Must succeed for rasters without NULLs
self.assertModule(module)

# Output should be identical in terms of NULL count
self.assertRasterFitsUnivar(
raster=self.mapComplete,
reference={"null_cells": float(0)},
precision=0,
Copy link
Member

@echoix echoix Feb 3, 2026

Choose a reason for hiding this comment

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

I don't think precision 0 is appropriate. It would mean a full unit of a number off would still pass. Is it that you had in mind?

Copy link
Contributor Author

@Abhi-d-gr8 Abhi-d-gr8 Feb 3, 2026

Choose a reason for hiding this comment

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

Thank you for the detailed investigation @echoix ! I understand your concern about precision=0, but I believe there might be a misunderstanding about what it means.

Looking at the values_equal function in checkers.py (lines 262-324), when precision=0:

For floats (lines 279-291), the code checks: if abs(value_a - value_b) > precision: Since both null_cells values are float(0), we get abs(0 - 0) > 0 which is False, so the values are considered equal.
For integers (lines 305-313), precision is only applied if int(precision) > 0. With precision=0, this condition is False, so it falls through to line 322 which does strict equality: elif value_a != value_b: return False

So precision=0 actually means "exact match required" with no tolerance, not "allow a difference of 1 unit".

Looking at the existing test suite (test_checkers.py), there are examples like:
Lines 290-295: precision=0 is used for exact integer comparisons
Line 42: values_equal(5, 8, precision=3) returns True (allows difference of 3)

In our case, we're checking that null_cells is exactly 0 (no tolerance), which is the correct behaviour for this test. Does this make sense, or am I missing something?

Copy link
Contributor

@nilason nilason Feb 3, 2026

Choose a reason for hiding this comment

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

0.0 ?

Copy link
Contributor Author

@Abhi-d-gr8 Abhi-d-gr8 Feb 3, 2026

Choose a reason for hiding this comment

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

Hi @nilason !
Are you suggesting I should use precision=0.0 instead of precision=0 to make it clearer that it's a float comparison? Or are you referring to the null_cells: float(0) value itself? Happy to change either for clarity!
If it's the first case then I saw codebase using 0 everywhere and I think python does auto converts it accordingly...

)

def test_bicubic(self):
"""Test using bicubic interpolation"""
module = SimpleModule(
Expand Down
Loading