From 70cab44a0f1d6d4b486143aad7c304286f2d9262 Mon Sep 17 00:00:00 2001 From: RAJVEER42 Date: Thu, 4 Jun 2026 01:44:10 +0530 Subject: [PATCH 1/3] r.vif: preserve active MASK when read_data() exits with an error Track the backup mask name in a module-level variable instead of CLEAN_RAST, and restore it from cleanup() before the temp-map removal loop so MASK survives crashes during r.stats / numpy processing. If a MASK already exists at restore time (left over from the internal r.mask call), remove it first; at cleanup time any MASK present is one r.vif created itself. Also wrap the r.stats/numpy block in try/finally so the internal mask is removed and the backup restored on the normal-exception path. Fixes #1702 --- src/raster/r.vif/r.vif.py | 90 +++++++++++++++++++++++++++++++-------- 1 file changed, 72 insertions(+), 18 deletions(-) diff --git a/src/raster/r.vif/r.vif.py b/src/raster/r.vif/r.vif.py index cda46d309e..682c1f594f 100755 --- a/src/raster/r.vif/r.vif.py +++ b/src/raster/r.vif/r.vif.py @@ -133,10 +133,49 @@ def prGreen(skk): CLEAN_RAST = [] +mask_backup_name = None + + +def restore_mask_backup(): + """Best-effort restore of MASK from the tracked backup. + + If a MASK already exists, it is one r.vif created internally (the script + held control throughout, so the user could not have set one). Remove it + before renaming the backup back. On success, clears mask_backup_name. + On failure, leaves it set and emits a warning with the backup name so the + user can restore manually. Never raises. + """ + global mask_backup_name + if mask_backup_name is None: + return + mapset = gs.gisenv()["MAPSET"] + found_backup = gs.find_file(name=mask_backup_name, element="cell", mapset=mapset) + if not found_backup["fullname"]: + mask_backup_name = None + return + found_mask = gs.find_file(name="MASK", element="cell", mapset=mapset) + if found_mask["fullname"]: + try: + gs.run_command("r.mask", flags="r", quiet=True) + except Exception: + pass # best effort; the rename below will fail with a clear warning + try: + gs.run_command( + "g.rename", raster=[mask_backup_name, "MASK"], quiet=True + ) + mask_backup_name = None + except Exception as exc: + gs.warning( + _( + "Failed to restore MASK from backup <{name}>: {err}. " + "Restore manually with: g.rename raster={name},MASK" + ).format(name=mask_backup_name, err=exc) + ) + def cleanup(): - """Remove temporary maps specified in the global list. In addition, - remove temporary files""" + """Restore the original MASK if needed, then remove temporary maps.""" + restore_mask_backup() cleanrast = list(reversed(CLEAN_RAST)) for rast in cleanrast: gs.run_command("g.remove", flags="f", type="all", name=rast, quiet=True) @@ -165,7 +204,9 @@ def check_layer(envlay): def read_data(raster, n, flag_s, seed): """Read in the raster layers as a numpy array.""" + global mask_backup_name gs.message("Reading in the data ...") + exist_mask = None if n: # Create mask random locations new_mask = tmpname("rvif") @@ -191,22 +232,35 @@ def read_data(raster, n, flag_s, seed): name="MASK", element="cell", mapset=gs.gisenv()["MAPSET"] ) if exist_mask["fullname"]: - mask_backup = tmpname("rvifoldmask") - gs.run_command("g.rename", raster=["MASK", mask_backup], quiet=True) - gs.run_command("r.mask", raster=new_mask, quiet=True) - - # Get the raster values at sample points - tmpcov = StringIO( - gs.read_command( - "r.stats", flags="1n", input=raster, quiet=True, separator="comma" - ).rstrip("\n") - ) - p = np.loadtxt(tmpcov, skiprows=0, delimiter=",") - - if n: - gs.run_command("r.mask", flags="r", quiet=True) - if exist_mask["fullname"]: - gs.run_command("g.rename", raster=[mask_backup, "MASK"], quiet=True) + # Track the backup in mask_backup_name (NOT CLEAN_RAST) so that + # cleanup() can restore it via restore_mask_backup() if the script + # exits with an error. Set only after a successful rename. + backup = create_unique_name("rvifoldmask") + gs.run_command("g.rename", raster=["MASK", backup], quiet=True) + mask_backup_name = backup + + try: + if n: + # Apply the random-sampling mask. If this raises, the finally + # block (and the atexit cleanup() as a backstop) restores MASK. + gs.run_command("r.mask", raster=new_mask, quiet=True) + # Get the raster values at sample points + tmpcov = StringIO( + gs.read_command( + "r.stats", flags="1n", input=raster, quiet=True, separator="comma" + ).rstrip("\n") + ) + p = np.loadtxt(tmpcov, skiprows=0, delimiter=",") + finally: + if n: + # Best-effort removal of the internal mask before restoring the + # user's MASK. Failures here are reported by restore_mask_backup(). + try: + gs.run_command("r.mask", flags="r", quiet=True) + except Exception: + pass + if exist_mask and exist_mask["fullname"]: + restore_mask_backup() return p From 89f04a4d3880ef21c00f043c4b619b261fc2538a Mon Sep 17 00:00:00 2001 From: RAJVEER42 Date: Fri, 3 Jul 2026 06:51:36 +0530 Subject: [PATCH 2/3] r.vif: apply ruff formatting --- src/raster/r.vif/r.vif.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/raster/r.vif/r.vif.py b/src/raster/r.vif/r.vif.py index 682c1f594f..64fefe9485 100755 --- a/src/raster/r.vif/r.vif.py +++ b/src/raster/r.vif/r.vif.py @@ -160,9 +160,7 @@ def restore_mask_backup(): except Exception: pass # best effort; the rename below will fail with a clear warning try: - gs.run_command( - "g.rename", raster=[mask_backup_name, "MASK"], quiet=True - ) + gs.run_command("g.rename", raster=[mask_backup_name, "MASK"], quiet=True) mask_backup_name = None except Exception as exc: gs.warning( From 388b953304a33fa6bf1e61340ef93a16dda88af1 Mon Sep 17 00:00:00 2001 From: RAJVEER42 Date: Sat, 4 Jul 2026 02:41:16 +0530 Subject: [PATCH 3/3] r.vif: use MaskManager on GRASS 8.5+ for the sampling mask On 8.5+ (MaskManager available), apply the random-sample mask via a MaskManager context. It restores the user's MASK automatically on exit, including on error, and removes the internal mask, so no manual backup or rename is needed there. The sample is drawn before entering the context so it still respects an active user MASK, matching the 8.4 path. The manual backup/restore path is kept for 8.4 compatibility, selected by trying to import grass.script.MaskManager. Addresses @wenzeslaus's review. --- src/raster/r.vif/r.vif.py | 109 +++++++++++++++++++++----------------- 1 file changed, 60 insertions(+), 49 deletions(-) diff --git a/src/raster/r.vif/r.vif.py b/src/raster/r.vif/r.vif.py index 64fefe9485..105c0e9617 100755 --- a/src/raster/r.vif/r.vif.py +++ b/src/raster/r.vif/r.vif.py @@ -121,6 +121,13 @@ from cStringIO import StringIO import grass.script as gs +try: + from grass.script import MaskManager + + HAS_MASK_MANAGER = True +except ImportError: + HAS_MASK_MANAGER = False + # Functions def prRed(skk): @@ -204,62 +211,66 @@ def read_data(raster, n, flag_s, seed): """Read in the raster layers as a numpy array.""" global mask_backup_name gs.message("Reading in the data ...") - exist_mask = None - if n: - # Create mask random locations - new_mask = tmpname("rvif") - if flag_s: - gs.run_command( - "r.random", - input=raster[0], - flags="s", - npoints=n, - raster=new_mask, - quiet=True, - ) - else: - gs.run_command( - "r.random", - input=raster[0], - seed=seed, - npoints=n, - raster=new_mask, - quiet=True, - ) - exist_mask = gs.find_file( - name="MASK", element="cell", mapset=gs.gisenv()["MAPSET"] - ) - if exist_mask["fullname"]: - # Track the backup in mask_backup_name (NOT CLEAN_RAST) so that - # cleanup() can restore it via restore_mask_backup() if the script - # exits with an error. Set only after a successful rename. - backup = create_unique_name("rvifoldmask") - gs.run_command("g.rename", raster=["MASK", backup], quiet=True) - mask_backup_name = backup - try: - if n: - # Apply the random-sampling mask. If this raises, the finally - # block (and the atexit cleanup() as a backstop) restores MASK. - gs.run_command("r.mask", raster=new_mask, quiet=True) - # Get the raster values at sample points + def read_sampled(): + """Read the raster values at the currently unmasked cells.""" tmpcov = StringIO( gs.read_command( "r.stats", flags="1n", input=raster, quiet=True, separator="comma" ).rstrip("\n") ) - p = np.loadtxt(tmpcov, skiprows=0, delimiter=",") + return np.loadtxt(tmpcov, skiprows=0, delimiter=",") + + def random_sample(out): + """Write a raster of n random sample points from raster[0] to .""" + kwargs = {"input": raster[0], "npoints": n, "raster": out, "quiet": True} + if flag_s: + kwargs["flags"] = "s" + else: + kwargs["seed"] = seed + gs.run_command("r.random", **kwargs) + + if not n: + return read_sampled() + + if HAS_MASK_MANAGER: + # GRASS 8.5+: draw the sample first so it respects any active user MASK + # (as on 8.4), then let MaskManager apply the sample as the mask for + # reading and restore the user's MASK automatically on exit, including + # on error. No manual backup or rename is needed. + sample = tmpname("rvif") + random_sample(sample) + with MaskManager() as manager: + gs.mapcalc( + "{m} = if(isnull({s}), null(), 1)".format( + m=manager.mask_name, s=sample + ), + quiet=True, + ) + return read_sampled() + + # GRASS 8.4: no MaskManager. Back up the user's MASK, apply the sample + # mask, and restore it in the finally block (with the atexit cleanup() as + # a backstop if the process is killed). + sample = tmpname("rvif") + random_sample(sample) + exist_mask = gs.find_file(name="MASK", element="cell", mapset=gs.gisenv()["MAPSET"]) + if exist_mask["fullname"]: + # Track the backup in mask_backup_name (NOT CLEAN_RAST) so cleanup() + # can restore it. Set only after a successful rename. + backup = create_unique_name("rvifoldmask") + gs.run_command("g.rename", raster=["MASK", backup], quiet=True) + mask_backup_name = backup + try: + gs.run_command("r.mask", raster=sample, quiet=True) + return read_sampled() finally: - if n: - # Best-effort removal of the internal mask before restoring the - # user's MASK. Failures here are reported by restore_mask_backup(). - try: - gs.run_command("r.mask", flags="r", quiet=True) - except Exception: - pass - if exist_mask and exist_mask["fullname"]: - restore_mask_backup() - return p + try: + gs.run_command("r.mask", flags="r", quiet=True) + except Exception: + pass + if exist_mask["fullname"]: + restore_mask_backup() def compute_vif(mapx, mapy):