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.mask.status: Check mask status through a tool #2390

Merged
merged 25 commits into from
Oct 11, 2024

Conversation

wenzeslaus
Copy link
Member

@wenzeslaus wenzeslaus commented May 20, 2022

Instead of using low-level test of file existence with hardcoded raster path and name, this offers a new tool to retrieve status of the raster mask.

The new r.mask.status tool reports presence or absence of the 2D raster mask and provides additional details about the mask.

There is one usage of this now and that's the one for a shell prompt. The prompt no longer relies on testing the file presence with the test program, but uses a GRASS tool to find out.

The code goes out of its way to report both mask name (currently always MASK) and the underlying raster name if it is a reclassified (without rewriting the current C API). This is to mimic the existing C functions which are returning the underlying raster if MASK is a reclass. The tool and the new C API function return both preparing a way for using an arbitrary name for the mask while having the option to look at the underlying reclassified raster map.

Instead of using low-level test of file existence with hardcoded raster path and name, use a module to retrieve status of the raster mask.

The new module r.mask.status reports presence or absence of the 2D raster mask and provides additional details about the mask.

The PR requires additional changes in the library which are not yet included.
@wenzeslaus wenzeslaus added this to the 8.4.0 milestone May 20, 2022
@wenzeslaus wenzeslaus added the C Related code is in C label May 20, 2022
@landam
Copy link
Member

landam commented Nov 20, 2023

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

@wenzeslaus wenzeslaus modified the milestones: 8.4.0, 8.5.0 Apr 26, 2024
@github-actions github-actions bot added raster Related to raster data processing Python Related code is in Python HTML Related code is in HTML libraries module docs labels Sep 9, 2024
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
raster/r.mask.status/main.c Outdated Show resolved Hide resolved
raster/r.mask.status/main.c Outdated Show resolved Hide resolved
raster/r.mask.status/main.c Outdated Show resolved Hide resolved
raster/r.mask.status/main.c Outdated Show resolved Hide resolved
@wenzeslaus
Copy link
Member Author

This is now ready for first round of reviews. I'm leaving it as a draft because I've left a lot of comments in the code considering other ways to implement this (I can move them to GitHub comments if desired).

There is one usage of this now and that's the one for a shell prompt. The prompt no longer relies on testing the file presence with the test program, but uses a GRASS tool to find out.

The code goes out of its way to report both mask name (always MASK) and the underlying raster name if it is a reclassified (without rewriting the current C API). This is to mimic the existing C functions which are returning the underlying raster if MASK is a reclass. The tool and the new C API function return both.

This is written with #2392 in mind trying to remove some of the details about 2D raster mask implementation, namely removing the name MASK from the API. This does not go as far as to resolve all the issues such as addressing the need for a simple mask presence test in #2390 or interactions with mask in Python tools which themselves need to use their own mask.

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.
…ainer'). The format option cannot be required otherwise running without parameters through the Python API does not work.
lib/raster/mask_info.c Outdated Show resolved Hide resolved
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.
@wenzeslaus wenzeslaus marked this pull request as ready for review October 11, 2024 15:21
@wenzeslaus
Copy link
Member Author

This is now ready for review. Example:

> r.mask.status format=json
{
    "present": true,
    "full_name": "MASK@tmp_8ed35ec319de47fcb50dfef2245fef08",
    "is_reclass_of": null
}
> r.mask -r
Raster MASK removed
> r.mask.status 
Mask is not present

lib/raster/mask_info.c Outdated Show resolved Hide resolved
Co-authored-by: Anna Petrasova <kratochanna@gmail.com>
petrasovaa
petrasovaa previously approved these changes Oct 11, 2024
raster/r.mask.status/tests/conftest.py Outdated Show resolved Hide resolved
raster/r.mask.status/tests/conftest.py Outdated Show resolved Hide resolved
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@wenzeslaus wenzeslaus merged commit 1d42e58 into OSGeo:main Oct 11, 2024
26 checks passed
@wenzeslaus wenzeslaus deleted the r_mask_status branch October 11, 2024 20:25
@wenzeslaus wenzeslaus changed the title r.mask.status: Check mask status through module r.mask.status: Check mask status through a tool Oct 11, 2024
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 Python Related code is in Python raster Related to raster data processing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants