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.buffer: Added test script #4482

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Changes from 4 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
126 changes: 126 additions & 0 deletions raster/r.buffer/testsuite/test_buffer.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
from grass.gunittest.case import TestCase
from grass.gunittest.main import test
from grass.gunittest.gmodules import SimpleModule
import grass.script as gs


class TestRBuffer(TestCase):

@classmethod
def setUpClass(cls):
cls.use_temp_region()
cls.runModule("g.region", raster="roadsmajor")

@classmethod
def tearDownClass(cls):
cls.del_temp_region()

def tearDown(self):
# Remove temporary maps created during tests
gs.run_command(
"g.remove",
type="raster",
name="null_map,zero_map,buf_test,buf_no_non_null,buf_ignore_zero",
flags="f",
)

def test_buffer_creation(self):
output = "buf_test"
distances = [100, 200, 300, 400, 500]

module = SimpleModule(
"r.buffer",
input="roadsmajor",
output=output,
distances=distances,
overwrite=True,
)
self.assertModule(module)

self.assertRasterExists(output)

expected_categories = [1] + [i + 1 for i in range(len(distances) + 1)]
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you want 1 twice?

Suggested change
expected_categories = [1] + [i + 1 for i in range(len(distances) + 1)]
expected_categories = [i + 1 for i in range(len(distances) + 1)]

Copy link
Author

Choose a reason for hiding this comment

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

the first 1 is the default category of the road, then i start i from i+1 because i already have a default 1 in the expected category list. The last +1 in len(distances) + 1 is because categories start from index 1.

I will conduct further tests to understand this in a deeper context, and apply corrections accordingly.

Copy link
Member

Choose a reason for hiding this comment

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

You can also look at the start and stop parameters of range if you'd feel it would make it simpler

Copy link
Author

Choose a reason for hiding this comment

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

Added the suggested change


self.assertRasterMinMax(
map=output,
refmin=min(expected_categories),
refmax=max(expected_categories),
msg=(
"Buffer zones should have category values from 1 to " # Checking if there are no abnormal values in the output raster map
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use f string here

+ str(max(expected_categories))
),
)

category_values = gs.read_command(
"r.stats", flags="n", input=output
).splitlines()
category_values = [int(line.split()[0]) for line in category_values]

print("Category values:", category_values)

unique_actual_categories = set(category_values)
self.assertTrue(
unique_actual_categories.issubset(set(expected_categories)),
msg=(
"Output categories should be a subset of expected categories: "
+ str(expected_categories)
),
)
Copy link
Contributor

Choose a reason for hiding this comment

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

If you want to test the exact categories, than the previous min/max test is somewhat redundant. Why subset, the categories should be compared exactly, no?

Copy link
Author

Choose a reason for hiding this comment

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

My thought process here was to first check if the categories are in the expected min max range. However, it doesn't verify if all expected categories are present or if there are extra categories.

Second check was to make sure there are no extra categories or abnormal categories that the user didn't want.

But I can see how it can be redundant. I will work on patching this and making sure a single test is sufficient.

Copy link
Author

Choose a reason for hiding this comment

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

Removed the redundant test


def test_no_non_null_values(self):
null_map = "null_map"
self.runModule("r.mapcalc", expression=f"{null_map} = null()")

output = "buf_no_non_null"
distances = [100, 200, 300]

module = SimpleModule(
"r.buffer",
input=null_map,
output=output,
distances=distances,
overwrite=True,
)
self.assertModule(module)

self.assertRasterExists(output)
stats = gs.read_command("r.univar", map=output, flags="g")
self.assertIn(
Copy link
Contributor

@petrasovaa petrasovaa Oct 10, 2024

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

In this test I use r.univar to ensure that the output map has no non-null cells when the input is entirely null. According to my understanding r.univar helps calculate univar stats and if it is a null output it gives out n=0 (number of non null cells). This verifies that the buffer operation correctly handles null input data without introducing any non-null values.

Copy link
Author

Choose a reason for hiding this comment

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

Used assertRasterFitsUnivar instead of manually doing it

"n=0", stats, msg="Output should have no non-NULL cells"
) # Checking an edge case where the input raster map is null

def test_ignore_zero_values(self):
zero_map = "zero_map"
self.runModule("r.mapcalc", expression=f"{zero_map} = if(row() % 2 == 0, 0, 1)")

output = "buf_ignore_zero"
distances = [100, 200]

module = SimpleModule(
"r.buffer",
input=zero_map,
output=output,
distances=distances,
flags="z",
overwrite=True,
)
self.assertModule(module)

self.assertRasterExists(output)

category_values = gs.read_command(
"r.stats", flags="n", input=output
).splitlines()
category_values = [int(line.split()[0]) for line in category_values]

print("Category values:", category_values)

self.assertNotIn(
Copy link
Contributor

Choose a reason for hiding this comment

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

This test is incorrect, it will pass even if category_values are empty. Could you better explain what exactly are you trying to test here? Also distances can be just 100, since given the input, it will never get to 200 meters...

Copy link
Author

Choose a reason for hiding this comment

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

I apologize for the multiple changes, I will be more careful with them. New to the team and still learning.
Will recheck these and push the changes soon.

Copy link
Author

Choose a reason for hiding this comment

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

The main idea here was to make a map with alternating rows of zero and one values. The first check is to see if it indeed made an output map and second is to verify there are no 0 values in the output as -z flag will help ignore zero values in output when creating buffers. This should verify that the -z flag works properly and confirms that buffer zones are not created around zero values and they are ignored.

I made a mistake in the assert. It should have been:
self.assertNotIn( 0, category_values, msg="Output should not contain buffer zones around zero values", )

Do you recommend keeping the test after making the above change and removing expected_categories?

Copy link
Contributor

Choose a reason for hiding this comment

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

There will never be 0 in the output, the output values always start with category 1 ("distances calculated from these locations"). So this test is not very useful as it is now. Perhaps you can create a raster with zeros and then run it with -z and then you should get an output raster with nulls only, because the zeroes are ignored and there are no source cells for the buffer, hence nulls everywhere. I recommend running it with different scenarios to make sure you understand what it's doing.

0,
Copy link
Contributor

Choose a reason for hiding this comment

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

r.buffer won't output 0 in any case, so it looks like it does not test what you want it to test.

Copy link
Author

Choose a reason for hiding this comment

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

I will work on patching it and diving deeper to understand the exact expected outputs in a few edge cases.

Copy link
Author

Choose a reason for hiding this comment

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

Compared with expected_categories = [1,2] instead of 0

category_values,
msg="Output should not contain buffer zones around zero values",
) # Check if the output raster map ignored 0 values


if __name__ == "__main__":
test()
Loading