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

raster: Read raster for mask from env variable #2392

Open
wants to merge 51 commits into
base: main
Choose a base branch
from

Conversation

wenzeslaus
Copy link
Member

@wenzeslaus wenzeslaus commented May 21, 2022

Use environment variable GRASS_MASK to obtain the name of the raster map to use as raster map for auto-masking.

Mask can be in another mapset. If it does not exist, mask is not applied. The direct masking rules for values (zero and null is masked out) apply (as when using g.copy raster=xxx,MASK).

It works with r.mask, i.e., r.mask gets the name from the environment variable. While setting and unsetting can be done only by manipulating the environment variable, r.mask will use whatever is the mask name set by the environment variable or the default name. This allows to fully use the r.mask capabilities without a need to reimplement them somewhere else. It will also keep the workflows with and without the variable same, so all the r.mask documentation applies to the advanced case of using the variable. If the mask set by the environment is in a different mapset, r.mask will fail.

An alternative would be to implement this without r.mask, i.e., r.mask does not get the name from the environment variable. Setting and unsetting this mask would be done only by manipulating the environment variable. This would be analogous to GRASS_REGION which cannot be manipulated by g.region and similar to g.copy used for MASK which cannot be mixed with r.mask -r. However, while GRASS_REGION says what the computational region should be, GRASS_MASK merely says what the mask name is regardless of its presence. We could fail if the raster is not present, but it seems more natural to behave the same as with the default name (MASK). This discussion then leads to the current implementation where r.mask respects whatever GRASS_MASK says.

r.fillnulls and r.in.wms are Python scripts which need to handle mask in a special way. The new functionality makes the implementation simpler and it makes the tools more robust as they don't touch the global mask state anymore. The new implementation is included here.

Review or revise code in this PR:

  • scripts/r.fillnulls/r.fillnulls.py (manipulates to handl mask in its own way)
  • scripts/r.in.wms/wms_base.py (manipulates to have custom mask)

Code changes implemented elsewhere:

Use variable:

  • testsuite/raster/raster_md5test.sh (in this PR)
  • raster/r.mapcalc/testsuite/const_map_test.sh (in this PR)

Use r.mask.status tool:

Update doc inside this PR:

  • doc/development/style_guide.md
  • raster/rasterintro.html
  • scripts/r.mask/r.mask.html
  • lib/raster/rasterlib.dox
  • comments in lib/raster source code

Update doc outside of this PR:

Changes needed:

Use environment variable GRASS_MASK to obtain the name of the raster map to use as raster map for auto-masking.

Mask can be in another mapset. If it does not exist, mask is not applied. The direct masking rules for values (zero and null is masked out) apply (as when using `g.copy raster=xxx,MASK`).

This is implemented as an alternative to using r.mask, i.e., r.mask does not get the name from the environment variable. Setting and unsetting is done by manipulating the environment variable. This is analogous to GRASS_REGION which cannot be manipulated by g.region and similar to g.copy used for MASK which cannot be mixed with r.mask -r.
@wenzeslaus wenzeslaus added this to the 8.4.0 milestone May 24, 2022
@wenzeslaus wenzeslaus added enhancement New feature or request C Related code is in C labels May 25, 2022
@wenzeslaus wenzeslaus modified the milestones: 8.3.0, 8.4.0 Feb 10, 2023
@landam
Copy link
Member

landam commented Nov 20, 2023

@wenzeslaus Please provide details on what is missing in order to review this PR?

@wenzeslaus wenzeslaus modified the milestones: 8.4.0, 8.5.0 Apr 26, 2024
wenzeslaus added a commit to wenzeslaus/grass that referenced this pull request Sep 27, 2024
On many places (more than covered here), 2D raster mask is called MASK conflating the concept (mask) and the implementation (MASK raster). Users using the r.mask tool may not interact with the underlying raster directly, so there is no reason to use MASK over mask.

This is leaving many places as they are with MASK. Some will be better revisited with or after OSGeo#2390 and OSGeo#2392 when a more comprehensive solution is available.

This fixes and keeps in sync wording r.null and r.external, and moves r.circle comment documenting the interface to a flag description.
wenzeslaus added a commit to wenzeslaus/grass that referenced this pull request Sep 27, 2024
To avoid asking about presence of the MASK raster, add a library function which checks for presence of the raster hiding its name in the library.

This prepares way for OSGeo#2392.

This also changes the message from using MASK to simply mask. I'm open to suggestion on wording of 'mask is present' versus 'mask is active' etc.
wenzeslaus added a commit that referenced this pull request Oct 11, 2024
On many places (more than covered here), 2D raster mask is called MASK conflating the concept (mask) and the implementation (MASK raster). Users using the r.mask tool may not interact with the underlying raster directly, so there is no reason to use MASK over mask.

This is changing MASK to mask and modifies related wording. However, this is also leaving many places as they are with MASK. Some will be better revisited with or after #2390 and #2392 when a more comprehensive solution is available.

This fixes and keeps in sync wording in r.null and r.external, and moves r.circle comment documenting the interface to a flag description.
@github-actions github-actions bot added raster Related to raster data processing HTML Related code is in HTML libraries module docs labels Oct 11, 2024
@github-actions github-actions bot added the tests Related to Test Suite label Oct 14, 2024
@wenzeslaus
Copy link
Member Author

If you are using the raster mask in scripting, now would be a good time to look at this PR, e.g., the documentation updates.

@wenzeslaus wenzeslaus marked this pull request as ready for review January 31, 2025 18:19
@wenzeslaus
Copy link
Member Author

This is now ready including documentation.

Set a custom mask inside a tool without modifying the mask set by user:

# Mask applies here (if any).
gs.run_command("r.slope.aspect", elevation=input_raster, aspect=aspect)

with gs.MaskManager():
    # Only the mask we set here will apply.
    gs.run_command("r.mask", raster=mask_raster)
    gs.run_command("r.slope.aspect", elevation=input_raster, slope=slope)
# Mask is disabled and the mask raster is removed at the end of the with block.

# Previously set mask applies here again.

It also simplifies existing tools, namely r.fillnulls and r.in.wms.

doc/development/style_guide.md Outdated Show resolved Hide resolved
doc/development/style_guide.md Outdated Show resolved Hide resolved
doc/development/style_guide.md Outdated Show resolved Hide resolved
gs.run_command("r.slope.aspect", elevation=input_raster, slope=slope)
```

To disable the mask, which may be needed in processing steps of import tool,
Copy link
Contributor

Choose a reason for hiding this comment

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

what do you mean here with import tool? all the examples are with r.slope.aspect

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe "an import tool". I could refer to r.in.wms as an example, but tgat relies an that code not changing. Making up an example seem like a lot theoretical code, but maybe the r.in.wms alpha is a good example. Appling a cloud mask would be another example.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, the "an" sounds good. Something like: "... in the processing steps of, e.g., an import tool, ..." to make it clear that it's just an example

doc/development/style_guide.md Outdated Show resolved Hide resolved
scripts/r.mask/r.mask.html Outdated Show resolved Hide resolved
scripts/r.mask/r.mask.html Outdated Show resolved Hide resolved
output, by naming the output raster 'MASK'. Such layers could have other
The <em>r.mask</em> function creates a mask with values 1 and NULL. But note
that a mask can also be created using other functions that have a raster as
output, by naming the output raster <code>MASK</code> (by default).
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
output, by naming the output raster <code>MASK</code> (by default).
output, by naming the output raster <code>MASK</code> (as in the default).

Comment on lines +61 to 67
in the mask map containing <code>NULL</code> or <code>0</code> will replace data with
NULL, while cells containing other values will allow data to pass through
unaltered. This means that:
<p>
If a binary map with [0,1] values is used as input in <em>r.mask</em>, all
raster cells with 0 and 1 will be part of the MASK. This is because
raster cells with 0 and 1 will be part of the mask. This is because
<em>r.mask</em> converts all non-NULL cells to 1.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this right? It sounds contradictory. In the first part it says that NULL or 0 will become NULL in the mask, while the second part says that 0 won´t become NULL in the mask...

Copy link
Member Author

Choose a reason for hiding this comment

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

Explaining the mask gets messy. Someone even put "it's counterintuitive" comment to the doc.

This sentence sounds wrong. I'll look into that. Maybe it is meant to say area which is masked out.

The masked, masked out terminology is not helping because the "presence" cells in the mask show rather than hide.

gs.fatal(
_(
"r.mask can only operate on raster mask in the current mapset "
"({current_mapset}), but mask name is set to {full_name}"
Copy link
Contributor

Choose a reason for hiding this comment

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

why "but"?

Copy link
Member Author

Choose a reason for hiding this comment

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

Full name will contain a different mapset than the current one, so the actual message should be more clear. Is that what you mean?

Copy link
Contributor

Choose a reason for hiding this comment

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

ah! yes, fine, I was only reading the text, not the code before it :)

wenzeslaus and others added 2 commits February 3, 2025 15:15
Co-authored-by: Veronica Andreo <veroandreo@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C Related code is in C docs enhancement New feature or request HTML Related code is in HTML libraries module notebook Python Related code is in Python raster Related to raster data processing tests Related to Test Suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants