Opened 13 years ago

Closed 12 years ago

#1310 closed defect (fixed)

Browse button in WxPython GUIs (for scripts) does not provide a correct file path for WinGRASS

Reported by: katrineggert1980 Owned by: grass-dev@…
Priority: critical Milestone: 6.4.2
Component: Shell Scripts Version: 6.4.1 RCs
Keywords: winGRASS Cc:
CPU: Unspecified Platform: MSWindows XP

Description

Greetings

I'm running GRASS script function (e.g. r.out.xyz) and If I use Browse button to search/define for output file it produces a path in Windows style (e.g. C:\data\GRASS\) that is wrong since GRASS's pythons do not use '\'. So user need to change '\' by hand to '/'. THis happens to all GRASS Scripts that uses Browse and file paths. This does not happen in text mode if we use " " around the file path.

Change History (27)

comment:1 by hellik, 13 years ago

tested with WinGRASS-6.4.SVN-r45749-1-Setup.exe

folder and file destination of the output files always definded by the Browse button of the wxgui

r.out.xyz input=elevation@PERMANENT output=C:\wd\exportgrass\routxyz.txt
c:/Program Files/GRASS 6.4.SVN/scripts/r.out.xyz: line 61:
C:wdexportgrassroutxyz.txt: Permission denied

but very interesting also tested with other raster and vector export modules

r.out.ascii input=elevation@PERMANENT output=C:\wd\exportgrass\routascii.asc    
(Sat Mar 26 14:43:54 2011) Command finished (10 sec) 
r.out.arc input=elevation@PERMANENT output=C:\wd\exportgrass\routarc.asc
(Sat Mar 26 14:46:38 2011) Command finished (12 sec)  
r.out.mat input=elevation@PERMANENT output=C:\wd\exportgrass\routmat.mat
r.out.mat komplett.
(Sat Mar 26 14:48:06 2011) Command finished (1 sec) 
r.out.png input=elevation@PERMANENT output=C:\wd\exportgrass\routpng.png
(Sat Mar 26 14:49:23 2011) Command finished (3 sec)
r.out.ppm input=elevation@PERMANENT output=C:\wd\exportgrass\routppm.ppm
Zeilen = 1350, Spalten = 1500
Konvertiere ...
r.out.ppm komplett. Datei <C:\wd\exportgrass\routppm.ppm> erstellt.
(Sat Mar 26 14:50:15 2011) Command finished (2 sec) 
r.out.gdal input=elevation@PERMANENT output=C:\wd\exportgrass\routgdal.gtif     
Exportiere in den GDAL-Datentyp: Float32
Prüfe GDAL Datentyp und NoData-Wert.
ERROR 6: SetColorTable() only supported for Byte or UInt16
bands in TIFF format.
Exportiere nach GDAL-Raster.
r.out.gdal komplett.
(Sat Mar 26 14:52:23 2011) Command finished (1 sec)  
v.out.ascii input=firestations@PERMANENT output=C:\wd\exportgrass\voutascii.txt 
(Sat Mar 26 14:53:32 2011) Command finished (0 sec)  
v.out.dxf input=firestations@PERMANENT output=C:\wd\exportgrass\voutdxf.dxf     
v.out.dxf komplett. 71 Feature geschrieben nach 'C:\wd\exportgrass\voutdxf.dxf'.
(Sat Mar 26 14:54:27 2011) Command finished (0 sec) 

are you able to test it on your side?

best regards Helmut

in reply to:  description ; comment:2 by hellik, 13 years ago

Replying to katrineggert1980: [...]

maybe some hints from the ML (http://lists.osgeo.org/pipermail/grass-dev/2011-March/053942.html)

Just a preliminary note: This error is reported here:
http://lists.osgeo.org/pipermail/grass-dev/2011-March/053774.html

I saw that Glynn sugeste this a couple of months ago:
http://lists.osgeo.org/pipermail/grass-dev/2010-October/052396.html

Can this be an option to solve this problem? Can anyone give a few tips on
this? (just to try and test it)
Thanks

Best Regards
Kat

in reply to:  1 ; comment:3 by neteler, 13 years ago

Replying to hellik:

tested with WinGRASS-6.4.SVN-r45749-1-Setup.exe

...

r.out.xyz input=elevation@PERMANENT output=C:\wd\exportgrass\routxyz.txt
c:/Program Files/GRASS 6.4.SVN/scripts/r.out.xyz: line 61:
C:wdexportgrassroutxyz.txt: Permission denied

Looking at the *script* r.out.xyz, we see that a redirection is used:

# if no output filename, output to stdout
if  [ -z "$GIS_OPT_OUTPUT" ] || [ "$GIS_OPT_OUTPUT" = "-" ]; then
    r.stats -1gn "$GIS_OPT_INPUT" fs="$GIS_OPT_FS"
else
    r.stats -1gn "$GIS_OPT_INPUT" fs="$GIS_OPT_FS" > "$GIS_OPT_OUTPUT"
fi

I suspect that this is causing problems on Windows.

in reply to:  3 ; comment:4 by neteler, 13 years ago

Replying to neteler:

Looking at the *script* r.out.xyz, we see that a redirection is used:

# if no output filename, output to stdout
if  [ -z "$GIS_OPT_OUTPUT" ] || [ "$GIS_OPT_OUTPUT" = "-" ]; then
    r.stats -1gn "$GIS_OPT_INPUT" fs="$GIS_OPT_FS"
else
    r.stats -1gn "$GIS_OPT_INPUT" fs="$GIS_OPT_FS" > "$GIS_OPT_OUTPUT"
fi

I suspect that this is causing problems on Windows.

I wonder why not simply using the "output=" parameter of r.stats:

  output   Name for output file (if omitted or "-" output to stdout)

Helmut, could you please test if adding output="$GIS_OPT_OUTPUT" helps?

in reply to:  4 ; comment:5 by hellik, 13 years ago

Replying to neteler:

Helmut, could you please test if adding output="$GIS_OPT_OUTPUT" helps?

tried with following:

# if no output filename, output to stdout
if  [ -z "$GIS_OPT_OUTPUT" ] || [ "$GIS_OPT_OUTPUT" = "-" ]; then
    r.stats -1gn "$GIS_OPT_INPUT" fs="$GIS_OPT_FS"
else
    r.stats -1gn "$GIS_OPT_INPUT" fs="$GIS_OPT_FS" > output="$GIS_OPT_OUTPUT"
fi

now there isn't any error message

r.out.xyz input=elevation@PERMANENT output=C:\wd\test\routxyzp.xyz              
(Sat Mar 26 18:30:37 2011) Command finished (46 sec)    

but the file doesn't seem to be written, the destination folder is empty.

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

Replying to hellik:

# if no output filename, output to stdout
if  [ -z "$GIS_OPT_OUTPUT" ] || [ "$GIS_OPT_OUTPUT" = "-" ]; then
    r.stats -1gn "$GIS_OPT_INPUT" fs="$GIS_OPT_FS"
else
    r.stats -1gn "$GIS_OPT_INPUT" fs="$GIS_OPT_FS" > output="$GIS_OPT_OUTPUT"
fi

but the file doesn't seem to be written, the destination folder is empty.

Yes, you forgot to take out ">". Try this code:

# if no output filename, output to stdout
if  [ -z "$GIS_OPT_OUTPUT" ] || [ "$GIS_OPT_OUTPUT" = "-" ]; then
    r.stats -1gn "$GIS_OPT_INPUT" fs="$GIS_OPT_FS" output="-"
else
    r.stats -1gn "$GIS_OPT_INPUT" fs="$GIS_OPT_FS" output="$GIS_OPT_OUTPUT"
fi

in reply to:  6 ; comment:7 by hellik, 13 years ago

Replying to neteler:

Yes, you forgot to take out ">". Try this code:

r.out.xyz input=elevation@PERMANENT output=C:\wd\test\test.xyz                  
ERROR: Kann Datei <C:wdtesttest.xyz> nicht zum Schreiben öffnen.
(Sat Mar 26 19:14:10 2011) Command finished (0 sec)   

so no change regarding the path issue.

Helmut

in reply to:  7 comment:8 by hellik, 13 years ago

Replying to hellik:

Replying to neteler:

Yes, you forgot to take out ">". Try this code:

r.out.xyz input=elevation@PERMANENT output=C:\wd\test\test.xyz                  
ERROR: Kann Datei <C:wdtesttest.xyz> nicht zum Schreiben öffnen.
(Sat Mar 26 19:14:10 2011) Command finished (0 sec)   

so no change regarding the path issue.

for the record with the original code:

if I copy the command from the wxgui to the wingrass-msys-shell

GRASS 6.4> r.out.xyz input=elevation@PERMANENT output=C:\wd\test\testroutxyz.txt
c:/Program Files/GRASS 6.4.SVN/scripts/r.out.xyz: line 61: C:wdtesttestroutxyz.txt: Permission denied

and with a quoted path in the wingrass-msys-shell

GRASS 6.4> r.out.xyz input=elevation@PERMANENT output="C:\wd\test\testroutxyz.txt"
 100%
GRASS 6.4> 

the original code is working in windows with a quoted path

in reply to:  2 ; comment:9 by hellik, 13 years ago

Replying to hellik:

from the dev-ML (http://lists.osgeo.org/pipermail/grass-dev/2011-March/053954.html)

katrin eggert wrote:

> Just a preliminary note: This error is reported here:
> http://lists.osgeo.org/pipermail/grass-dev/2011-March/053774.html
> 
> I saw that Glynn sugeste this a couple of months ago:
> http://lists.osgeo.org/pipermail/grass-dev/2010-October/052396.html
> 
> Can this be an option to solve this problem? Can anyone give a few tips on
> this? (just to try and test it)

Right. Currently, 6.5 has utils.split(), which handles Windows
filenames, but much of the wx GUI is still using shlex.split()
directly.

Feel free to replace all occurrences of shlex.split() with
utils.split() in your local copy. Also, open a bug report (if there
isn't already one for this issue).

comment:10 by neteler, 13 years ago

To change the code in automated way:

cd gui/wxpython/gui_modules/
for i in menuform.py gmodeler.py menu.py prompt.py goutput.py ; do mv
$i tmp.py ; cat tmp.py | sed 's+shlex.split+utils.split+g' > $i ; rm
-f tmp.py ; done
for i in menuform.py gmodeler.py menu.py prompt.py goutput.py ; do mv
$i tmp.py ; cat tmp.py | sed 's+import shlex+import utils+g' > $i ; rm
-f tmp.py ; done

in reply to:  9 ; comment:11 by martinl, 13 years ago

Replying to hellik:

> katrin eggert wrote:
> 
> > Just a preliminary note: This error is reported here:
> > http://lists.osgeo.org/pipermail/grass-dev/2011-March/053774.html
> > 
> > I saw that Glynn sugeste this a couple of months ago:
> > http://lists.osgeo.org/pipermail/grass-dev/2010-October/052396.html
> > 
> > Can this be an option to solve this problem? Can anyone give a few tips on
> > this? (just to try and test it)
> 
> Right. Currently, 6.5 has utils.split(), which handles Windows
> filenames, but much of the wx GUI is still using shlex.split()
> directly.
> 
> Feel free to replace all occurrences of shlex.split() with
> utils.split() in your local copy. Also, open a bug report (if there
> isn't already one for this issue).

done in r45770 (devbr6) and r45771 (trunk). Testing welcomed.

in reply to:  11 comment:12 by hellik, 13 years ago

Replying to martinl:

done in r45770 (devbr6) and r45771 (trunk). Testing welcomed.

tested in a self compiled wingrass65 r45770

r.out.xyz --verbose input=elevation@PERMANENT output=C:\wd\test\wingrass65rout.xyz
c:/osgeo4w/usr/src/grass6_devel/dist.i686-pc-
mingw32/scripts/r.out.xyz: line 61:
C:wdtestwingrass65rout.xyz: Permission denied

comment:13 by katrineggert1980, 13 years ago

I have tested with the following:

in reply to:  13 comment:14 by katrineggert1980, 13 years ago

Replying to katrineggert1980:

I have tested with the following:

Ups Sorry. This meant before these updates :)

comment:15 by katrineggert1980, 13 years ago

I have installed WinGRASS6.4 night built from today (28th March 2011) and I tried: (Mon Mar 28 10:16:51 2011) r.out.xyz input=lsat5_1987_10@landsat output=C:\delete\tried\output01 fs= (Mon Mar 28 10:16:53 2011) Command finished (1 sec)

It didn't created the file in C:\delete\tried\output01. Again, it has ignored the bars... So it's still not working

comment:16 by katrineggert1980, 13 years ago

Hi I have just tried Nightly built for GRASS6.5 and: (Mon Mar 28 10:31:48 2011) r.out.xyz input=lsat5_1987_10@landsat output=C:\delete\tried\output02 fs= path: C:deletetriedoutput02 Output written to C:deletetriedoutput02 Output written to C:deletetriedoutput02 (Mon Mar 28 10:31:50 2011) Command finished (1 sec)

And now it«s not even creating the file c:\deletetriedoutput02

I added, in the beggining of the code a print of $GIS_OPT_OUTPUT and it's printed as: C:deletetriedoutput02

So it's still not working

in reply to:  16 comment:17 by martinl, 13 years ago

Replying to katrineggert1980:

I added, in the beggining of the code a print of $GIS_OPT_OUTPUT and it's printed as: C:deletetriedoutput02

So it's still not working

source of the problem are bat scripts, in this case $ARCH_DISTDIR\bin\r.out.xyz.bat

@"%GRASS_SH%" -c '"%GISBASE%/scripts/r.out.xyz" %*'

where '\` are escaped. To make Bash script working on Windows is generally speaking pain. The future are Python scripts in GRASS 7. I still think that Pythonized script could replace most of Bash scripts in GRASS 6.

Benefits:

  • maintaining scripts only in one language
  • Python scripts can run on Windows without any problems

Drawback:

  • some new bugs can be introduced, anyway some Bash-related can be fixed by this conversion
  • Python as requirement

I proposed that some time ago, but it was refused.

comment:18 by katrineggert1980, 13 years ago

Ok it makes some sense. If I try to run anything on Msys the \ are ignored. So this can be a possible solution. But my question is, why C Modules accepts "\" and Scripts don't.

in reply to:  18 comment:19 by martinl, 13 years ago

Replying to katrineggert1980:

Ok it makes some sense. If I try to run anything on Msys the \ are ignored. So this can be a possible solution. But my question is, why C Modules accepts "\" and Scripts don't.

Because C modules don't use Bash for parsing the arguments. I would say rather *Bash* scripts, Python scripts haven't such problem.

comment:20 by martinl, 13 years ago

Component: wxGUIBash

Changing compoment to Bash.

Running from msys

r.out.xyz input=elevation out=C:\Temp\x.txt

create Tempx.txt in C drive.

 r.out.xyz input=elevation out="C:\Temp\x.txt"

works as expected.

comment:21 by katrineggert1980, 13 years ago

Just an idea to fix this (for now and for Win users): is it possible to change wxGUI parsing values in order to automatically change "\" (windows paths) to "/" before it sends the options to Bash?

in reply to:  21 comment:22 by martinl, 13 years ago

Replying to katrineggert1980:

Just an idea to fix this (for now and for Win users): is it possible to change wxGUI parsing values in order to automatically change "\" (windows paths) to "/" before it sends the options to Bash?

it's just a tricky workaround, let's find a better solution. Since we are still keeping Bash script sin GRASS 6 we need to find a solution... Any idea?

comment:23 by katrineggert1980, 13 years ago

A file path that is inserted in WxGUI is first parsed in grass.parser() So I went to core.py to parser() and, if I print for instance print (sys.argv[2]) my file path is no longer with '\' So the options in sys.argv (line 422) are no longer with '\' So my question is, what function places data and information in sys.argv variable?

comment:24 by toyzerocha, 13 years ago

As far as I can see, the only solution seems to be in *.bat file. I mean, when the command is sent to GRASS_SH something should be processed before in order to switch the bars. Any idea? (I don't much about Bash Scripting)

comment:25 by hamish, 12 years ago

Milestone: 6.4.16.4.2
Summary: Browse button in WxPython GUIs (for scripts) does not provide a correct file path for WinGRASBrowse button in WxPython GUIs (for scripts) does not provide a correct file path for WinGRASS

fixed by r48449? (duplicate of #1447?)

scripts/windows_launch.bat now reads:

@"%GRASS_SH%" "%GISBASE%/scripts/SCRIPT_NAME" %*

(no more -c, 'single quotes' removed)

Hamish

in reply to:  25 ; comment:26 by hellik, 12 years ago

Replying to hamish:

fixed by r48449? (duplicate of #1447?)

scripts/windows_launch.bat now reads:

@"%GRASS_SH%" "%GISBASE%/scripts/SCRIPT_NAME" %*

(no more -c, 'single quotes' removed)

tested with

osgeo4w-wingrass6.4.3 r52715

r.out.xyz --verbose input=elevation@PERMANENT output=C:\tmp\elevoutwingrass643.txt
(Sat Aug 18 15:22:48 2012) Befehl ausgeführt (10 sec)

osgeo4w-wingrass r52495

r.out.xyz --verbose input=elevation@PERMANENT output=C:\tmp\elevoutwingrass65.txt
(Sat Aug 18 15:26:16 2012) Befehl ausgeführt (7 sec)

closing ticket?

Helmut

in reply to:  26 comment:27 by hellik, 12 years ago

Resolution: fixed
Status: newclosed

Replying to hellik:

closing ticket?

closing, reopen if needed.

Helmut

Note: See TracTickets for help on using tickets.