Opened 7 years ago

Closed 3 years ago

#1804 closed defect (fixed)

r.clump NULL handling

Reported by: marisn Owned by: grass-dev@…
Priority: normal Milestone: 7.0.5
Component: Raster Version: svn-trunk
Keywords: r.clump Cc:
CPU: Unspecified Platform: Unspecified

Description

Current r.clump implementation uses 0 as NULL indicator and NULL cells as any other values.

Setting 0 and NULL to NULL can be done with following code. Unfortunately today my pointer-fu is too weak (around line 229) to change code to treat 0 as valid value and leave NULL as-is.

Clumping 0 areas as any other areas would be sensible thing. The open question remains - should NULL areas also be clumped or just skipped and left as-is (NULL)?

Index: raster/r.clump/clump.c
===================================================================
--- raster/r.clump/clump.c      (revision 53904)
+++ raster/r.clump/clump.c      (working copy)
@@ -105,7 +105,7 @@
            for (col = 1; col <= ncols; col++) {
                LEFT = X;
                X = cur_in[col];
-               if (X == 0) {   /* don't clump zero data */
+               if (X == 0 || Rast_is_c_null_value(&X)) { /* don't clump zero/NULL data */
                    cur_clump[col] = 0;
                    continue;
                }

Change History (8)

comment:1 Changed 7 years ago by martinl

Keywords: r.clump added

comment:2 in reply to:  description ; Changed 7 years ago by mmetz

Replying to marisn:

Current r.clump implementation uses 0 as NULL indicator and NULL cells as any other values.

You could try r.clump2 from addons [0]. It should do what you want.

Markus M

[0] http://grasswiki.osgeo.org/wiki/AddOns/GRASS7/raster#r.clump2

comment:3 in reply to:  2 Changed 7 years ago by neteler

Replying to mmetz:

You could try r.clump2 from addons [0]. It should do what you want.

We discussed in long time ago in the office :-) Maybe replace r.clump in trunk with r.clump2 from Addons which is so much nicer?

comment:4 Changed 6 years ago by neteler

Fixes have been made in r59076, r59082, r59122, r59128, r59131, r59136 but I don't know if r.clump2 in Addons is still better. The first fixes "do not clump NULL data".

Solved?

comment:5 in reply to:  4 Changed 6 years ago by mmetz

Replying to neteler:

Fixes have been made in r59076, r59082, r59122, r59128, r59131, r59136 but I don't know if r.clump2 in Addons is still better. The first fixes "do not clump NULL data".

r.clump2 did not provide all the functionality of r.clump and was slower than the new r.clump; removed from addons.

comment:6 Changed 3 years ago by martinl

Milestone: 7.0.07.0.5

comment:7 Changed 3 years ago by neteler

Anything left here, marisn?

comment:8 in reply to:  7 Changed 3 years ago by marisn

Resolution: fixed
Status: newclosed

Replying to neteler:

Anything left here, marisn?

There have been made serious improvements for the module thus it seems to be completely fixed for me.

Note: See TracTickets for help on using tickets.