Opened 6 years ago

Closed 6 years ago

#2116 closed defect (fixed)

r.kappa: call to r.stats fails if spaces in path

Reported by: mlennert Owned by: grass-dev@…
Priority: normal Milestone: 6.4.4
Component: Raster Version: svn-releasebranch64
Keywords: r.kappa, path Cc:
CPU: Unspecified Platform: Unspecified

Description (last modified by hamish)

To reproduce with GISDBASE="GRASS DATA":

r.kappa classification=landclass96@PERMANENT reference=landuse96_28m@PERMANENT

Sorry <DATA/nc_spm_08/user1/.tmp/moritz/31976.0> is not a valid option

The attached patch solves the problem for me, but I imagine that there is a more elegant solution.

Moritz

Attachments (1)

kappa_rstats.diff (396 bytes) - added by mlennert 6 years ago.
patch for grass64release

Download all attachments as: .zip

Change History (8)

Changed 6 years ago by mlennert

Attachment: kappa_rstats.diff added

patch for grass64release

comment:1 in reply to:  description Changed 6 years ago by hamish

Replying to mlennert:

The attached patch solves the problem for me, but I imagine that there is a more elegant solution.

right, we should use G_vspawn_ex() instead of system(). It's already done for r.kappa in G7; just needs to be backported & tested. see r40146 and r44964.

In addition, as a minor style tweak/simplification I'd suggest to consider using the r.stats output= file option instead of a shell redirect/dealing with the stdout pipe.

regards, Hamish

comment:2 Changed 6 years ago by hamish

on the other hand, for choosing the path of least change in the stable branch your patch might be the way to go.

comment:3 Changed 6 years ago by hamish

Component: DefaultRaster
Description: modified (diff)
Keywords: r.kappa added; kappa removed

simple quoting fix committed in devbr6 & relbr64 with r58111,2.

it should be tested in wingrass before closing the ticket, see the i.smap help page for an example.

Hamish

comment:4 in reply to:  3 ; Changed 6 years ago by mlennert

Replying to hamish:

simple quoting fix committed in devbr6 & relbr64 with r58111,2.

it should be tested in wingrass before closing the ticket, see the i.smap help page for an example.

Thanks a lot ! I hadn't applied the patch, yet, since you suggested G_vspawn_ex(), and I didn't find the time to look into that. Should that solution be preferred ? Should we maybe leave the ticket open as a reminder, or should we just say that if it works in windows we're happy with this quick fix for grass6 ?

Moritz

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

Replying to mlennert:

I hadn't applied the patch, yet, since you suggested G_vspawn_ex(), and I didn't find the time to look into that. Should that solution be preferred ?

The spawn functions are preferable (that's what r.kappa uses in 7.0), as they avoid the shell altogether.

comment:6 in reply to:  5 ; Changed 6 years ago by neteler

Replying to glynn:

Replying to mlennert:

I hadn't applied the patch, yet, since you suggested G_vspawn_ex(), and I didn't find the time to look into that. Should that solution be preferred ?

The spawn functions are preferable (that's what r.kappa uses in 7.0), as they avoid the shell altogether.

Backported to 6.5 in r60121 and to 6.4 in r60122. The result is identical 7.0.

Please test on Windows and close if solved.

comment:7 in reply to:  6 Changed 6 years ago by mlennert

Resolution: fixed
Status: newclosed

Replying to neteler:

Replying to glynn:

Replying to mlennert:

I hadn't applied the patch, yet, since you suggested G_vspawn_ex(), and I didn't find the time to look into that. Should that solution be preferred ?

The spawn functions are preferable (that's what r.kappa uses in 7.0), as they avoid the shell altogether.

Backported to 6.5 in r60121 and to 6.4 in r60122. The result is identical 7.0.

Please test on Windows and close if solved.

Just tested on Windows 7 Virtual Machine with WinGRASS-6.4.4svn-r60212-962-Setup.exe (yesterday's build) and GISDBASE containing "GRASS DATA": it seems to work perfectly.

Closing.

Moritz

Note: See TracTickets for help on using tickets.