[GRASS-dev] [GRASS GIS] #2314: output r.out.xzy

GDAL · May 27, 2014, 9:01am

#2314: output r.out.xzy
-------------------------+--------------------------------------------------
Reporter: pvanbosgeo | Owner: grass-dev@…
Type: defect | Status: new
Priority: normal | Milestone: 7.0.0
Component: Default | Version: unspecified
Keywords: | Platform: Unspecified
Cpu: Unspecified |
-------------------------+--------------------------------------------------
According to the help file,

separator=character
     Field separator
     Special characters: newline, space, comma, tab
     Default: |

However, in the output it seems a space rather than | is used by default.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2314>
GRASS GIS <http://grass.osgeo.org>

GDAL · May 27, 2014, 9:08am

#2314: output r.out.xzy
-------------------------+--------------------------------------------------
Reporter: pvanbosgeo | Owner: grass-dev@…
Type: defect | Status: new
Priority: normal | Milestone: 7.0.0
Component: Default | Version: unspecified
Keywords: | Platform: Unspecified
Cpu: Unspecified |
-------------------------+--------------------------------------------------

Comment(by pvanbosgeo):

In addition, the separator | is not accepted at all. Using it will result
in an output without separator.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2314#comment:1>
GRASS GIS <http://grass.osgeo.org>

GDAL · May 29, 2014, 12:17am

#2314: output r.out.xzy
-------------------------+--------------------------------------------------
Reporter: pvanbosgeo | Owner: grass-dev@…
Type: defect | Status: new
Priority: normal | Milestone: 7.0.0
Component: Default | Version: unspecified
Keywords: | Platform: Unspecified
Cpu: Unspecified |
-------------------------+--------------------------------------------------

Comment(by hcho):

I confirmed this issue. Related to r57905. Not sure why the pipe character
was disabled.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2314#comment:2>
GRASS GIS <http://grass.osgeo.org>

GDAL · May 29, 2014, 12:31am

#2314: output r.out.xzy
-------------------------+--------------------------------------------------
Reporter: pvanbosgeo | Owner: grass-dev@…
Type: defect | Status: new
Priority: normal | Milestone: 7.0.0
Component: Default | Version: unspecified
Keywords: | Platform: Unspecified
Cpu: Unspecified |
-------------------------+--------------------------------------------------

Comment(by hcho):

Fixed the usage and displays a warning when | is used. (r60569)

annakrat: Is the | issue on Windows only with python scripts and not with
binary modules? What exactly is the problem?

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2314#comment:3>
GRASS GIS <http://grass.osgeo.org>

GDAL · May 29, 2014, 11:28pm

#2314: output r.out.xyz
-------------------------+--------------------------------------------------
Reporter: pvanbosgeo | Owner: grass-dev@…
Type: defect | Status: new
Priority: normal | Milestone: 7.0.0
Component: Default | Version: unspecified
Keywords: | Platform: Unspecified
Cpu: Unspecified |
-------------------------+--------------------------------------------------

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2314#comment:4>
GRASS GIS <http://grass.osgeo.org>

GDAL · May 29, 2014, 11:30pm

#2314: output r.out.xyz
-------------------------+--------------------------------------------------
Reporter: pvanbosgeo | Owner: grass-dev@…
Type: defect | Status: new
Priority: normal | Milestone: 7.0.0
Component: Default | Version: unspecified
Keywords: | Platform: Unspecified
Cpu: Unspecified |
-------------------------+--------------------------------------------------

Comment(by glynn):

Replying to [comment:2 hcho]:
> I confirmed this issue. Related to r57905. Not sure why the pipe
character was disabled.
r60591 reverts r57905 and r60569. Vertical bar ("pipe") character is again
allowed, and is the default.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2314#comment:5>
GRASS GIS <http://grass.osgeo.org>

GDAL · May 30, 2014, 12:57am

#2314: output r.out.xyz
-------------------------+--------------------------------------------------
Reporter: pvanbosgeo | Owner: grass-dev@…
Type: defect | Status: new
Priority: normal | Milestone: 7.0.0
Component: Default | Version: unspecified
Keywords: | Platform: Unspecified
Cpu: Unspecified |
-------------------------+--------------------------------------------------

Comment(by annakrat):

Replying to [comment:3 hcho]:
> Fixed the usage and displays a warning when | is used. (r60569)
>
> annakrat: Is the | issue on Windows only with python scripts and not
with binary modules? What exactly is the problem?

Here is the problem a user had with running r.out.xyz:

http://lists.osgeo.org/pipermail/grass-user/2013-October/069019.html

I remember having these problems with pipe as separator when calling a
module from python script on Windows. It seemed that parsing of the
command failed and it tried to evaluate whatever was after pipe as another
command. So I think, reverting might break it again but we can see
tomorrow if this issue is still there.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2314#comment:6>
GRASS GIS <http://grass.osgeo.org>

GDAL · May 30, 2014, 2:57am

#2314: output r.out.xyz
------------------------+---------------------------------------------------
Reporter: pvanbosgeo | Owner: grass-dev@…
Type: defect | Status: new
Priority: normal | Milestone: 7.0.0
Component: Default | Version: svn-trunk
Keywords: | Platform: MSWindows 7
Cpu: All |
------------------------+---------------------------------------------------
Changes (by hcho):

  * platform: Unspecified => MSWindows 7
  * version: unspecified => svn-trunk
  * cpu: Unspecified => All

Comment:

I didn't try on Windows, but I think this is a problem with wxGUI, not
with r.out.xyz, so we should fix the wxGUI console. We cannot work around
this issue for all other modules that have a separator option. Maybe,
double quotes around a pipe?

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2314#comment:7>
GRASS GIS <http://grass.osgeo.org>

GDAL · May 30, 2014, 7:27am

#2314: output r.out.xyz
------------------------+---------------------------------------------------
Reporter: pvanbosgeo | Owner: grass-dev@…
Type: defect | Status: new
Priority: normal | Milestone: 7.0.0
Component: Default | Version: svn-trunk
Keywords: | Platform: MSWindows 7
Cpu: All |
------------------------+---------------------------------------------------

Comment(by pvanbosgeo):

May I offer a suggestion as an user? Disabling hitherto default parameter
settings are fairly important as they will break user-made scripts. It did
for me, and it wasn't easy to find out why they didn't work anymore. More
so because the | was still listed as the default in the help files.

So, whenever such changes are made, it would be good to change it in the
help file and/or put them on a list like
http://trac.osgeo.org/grass/wiki/Grass7/NewFeatures (but perhaps a
separate list for the development version, don't know).

I am fully aware that time may be an impediment (I am very grateful for
all the time you are already putting into improving the software), and I
know that by using the development version I run the risk of things
breaking, but still, if possible such changes in the help file could be
helpful and also make it easier to spot potential bugs, etc.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2314#comment:8>
GRASS GIS <http://grass.osgeo.org>

GDAL · May 30, 2014, 7:32am

#2314: output r.out.xyz
------------------------+---------------------------------------------------
Reporter: pvanbosgeo | Owner: grass-dev@…
Type: defect | Status: new
Priority: normal | Milestone: 7.0.0
Component: Default | Version: svn-trunk
Keywords: | Platform: MSWindows 7
Cpu: All |
------------------------+---------------------------------------------------

Comment(by pvanbosgeo):

As an additional comment, wouldn't it be possible to simply change the
default, but keeping the | as an option? I for one always declare all
parameters in my scripts, even if I am using the default settings,
precisely to avoid problems if ever the default is changed. Of course,
that doesn't help when some options are removed altogether.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2314#comment:9>
GRASS GIS <http://grass.osgeo.org>

GDAL · May 30, 2014, 11:54am

#2314: output r.out.xyz
------------------------+---------------------------------------------------
Reporter: pvanbosgeo | Owner: grass-dev@…
Type: defect | Status: new
Priority: normal | Milestone: 7.0.0
Component: Default | Version: svn-trunk
Keywords: | Platform: MSWindows 7
Cpu: All |
------------------------+---------------------------------------------------

Comment(by annakrat):

Replying to [comment:8 pvanbosgeo]:
> May I offer a suggestion as an user? Disabling hitherto default
parameter settings are fairly important as they will break user-made
scripts. It did for me, and it wasn't easy to find out why they didn't
work anymore. More so because the | was still listed as the default in the
help files.
>
Sorry for that but the intention wasn't to disable it but to fix it
(although it was more a workaround than fixing). Now I see what was wrong,
I expected r.stats, the underlying module, to have | as default separator
parameter but it has space instead (btw is there any reason for this
inconsistency?). My intention was to not include | in the parameters with
which r.stats is called (because that was causing problem) and instead
make r.stats use its default separator.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2314#comment:10>
GRASS GIS <http://grass.osgeo.org>

GDAL · May 30, 2014, 12:08pm

#2314: output r.out.xyz
------------------------+---------------------------------------------------
Reporter: pvanbosgeo | Owner: grass-dev@…
Type: defect | Status: new
Priority: normal | Milestone: 7.0.0
Component: Default | Version: svn-trunk
Keywords: | Platform: MSWindows 7
Cpu: All |
------------------------+---------------------------------------------------

Comment(by pvanbosgeo):

Thanks for the explanation and apologies for my wrong interpretation

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2314#comment:11>
GRASS GIS <http://grass.osgeo.org>

GDAL · May 30, 2014, 2:19pm

#2314: output r.out.xyz
------------------------+---------------------------------------------------
Reporter: pvanbosgeo | Owner: grass-dev@…
Type: defect | Status: new
Priority: normal | Milestone: 7.0.0
Component: Default | Version: svn-trunk
Keywords: | Platform: MSWindows 7
Cpu: All |
------------------------+---------------------------------------------------

Comment(by annakrat):

Replying to [comment:7 hcho]:
> I didn't try on Windows, but I think this is a problem with wxGUI, not
with r.out.xyz, so we should fix the wxGUI console. We cannot work around
this issue for all other modules that have a separator option. Maybe,
double quotes around a pipe?

It's not problem of wxGUI but maybe of Python scripting library. Today's
version of r.out.xyz on Windows gives this error when you specify | or you
leave default:

{{{
The syntax of the command is incorrect.
}}}
Adding quotes around pipe (for example in ```_make_val``` function in
grass.core) works but results in ```"|"``` as a separator (3 characters)
which is wrong. So, now we have r.out.xyz broken on Windows. Any ideas?

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2314#comment:12>
GRASS GIS <http://grass.osgeo.org>

GDAL · May 30, 2014, 2:36pm

#2314: output r.out.xyz
-------------------------------------------------+--------------------------
Reporter: pvanbosgeo | Owner: grass-dev@…
Type: defect | Status: new
Priority: normal | Milestone: 7.0.0
Component: Default | Version: svn-trunk
Keywords: separator, pipe, r.out.xyz, r.stats | Platform: MSWindows 7
Cpu: All |
-------------------------------------------------+--------------------------
Changes (by wenzeslaus):

* keywords: => separator, pipe, r.out.xyz, r.stats

Comment:

Replying to [comment:12 annakrat]:
> Any ideas?

  * Use `separator=pipe` instead of `separator=|`, similarly to
`separator=space`.
  * Fix G7:r.stats, the default separator for all other modules is a pipe.
(And than apply again the patch.)
  * Another idea would be to use some other separator by default which
would discourage usage of pipe by MS Windows users (which anyway must be
very confused from that character, I would say). A space or comma could be
much natural separators (e.g., considering putting the data to some
spreadsheet or computational environment such as R). This does not solve
the actual problem but it might completely avoid it on MS Windows and user
of other systems can still use the pipe if they want.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2314#comment:13>
GRASS GIS <http://grass.osgeo.org>

GDAL · May 30, 2014, 4:09pm

#2314: output r.out.xyz
-------------------------------------------------+--------------------------
Reporter: pvanbosgeo | Owner: grass-dev@…
Type: defect | Status: new
Priority: normal | Milestone: 7.0.0
Component: Default | Version: svn-trunk
Keywords: separator, pipe, r.out.xyz, r.stats | Platform: MSWindows 7
Cpu: All |
-------------------------------------------------+--------------------------

Comment(by hcho):

separator=pipe is added in r60614. G_OPT_F_SEP defaults to "pipe", not to
"|". I think it should fix the problem.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2314#comment:14>
GRASS GIS <http://grass.osgeo.org>

GDAL · May 31, 2014, 4:55am

#2314: output r.out.xyz
-------------------------------------------------+--------------------------
Reporter: pvanbosgeo | Owner: grass-dev@…
Type: defect | Status: new
Priority: normal | Milestone: 7.0.0
Component: Default | Version: svn-trunk
Keywords: separator, pipe, r.out.xyz, r.stats | Platform: MSWindows 7
Cpu: All |
-------------------------------------------------+--------------------------

Comment(by glynn):

Replying to [comment:12 annakrat]:

> It's not problem of wxGUI but maybe of Python scripting library. Today's
version of r.out.xyz on Windows gives this error when you specify | or you
leave default:
>
{{{
The syntax of the command is incorrect.
}}}

How are you running the script?

If it isn't via cmd.exe in a console window, with parameters provided on
the command line, then that's the first thing to try. Any problems when
executing via wxGUI or using the parameter dialogs should be assumed to be
problems with those features until proven otherwise.

> Adding quotes around pipe (for example in ```_make_val``` function in
grass.core) works but results in ```"|"``` as a separator (3 characters)
which is wrong. So, now we have r.out.xyz broken on Windows. Any ideas?

If a command is executed via e.g. grass.run_command(), Python's
subprocess.Popen() provides the necessary quoting.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2314#comment:15>
GRASS GIS <http://grass.osgeo.org>

GDAL · May 31, 2014, 5:12am

#2314: output r.out.xyz
-------------------------------------------------+--------------------------
Reporter: pvanbosgeo | Owner: grass-dev@…
Type: defect | Status: new
Priority: normal | Milestone: 7.0.0
Component: Default | Version: svn-trunk
Keywords: separator, pipe, r.out.xyz, r.stats | Platform: MSWindows 7
Cpu: All |
-------------------------------------------------+--------------------------

Comment(by glynn):

Replying to [comment:14 hcho]:
> separator=pipe is added in r60614. G_OPT_F_SEP defaults to "pipe", not
to "|". I think it should fix the problem.

It adds a few new ones, namely that Python scripts (which don't use
G_option_to_separator()) are now trying to use the literal string "pipe"
as a separator. E.g.
{{{
$ echo "170.510125 -45.868537" | m.proj -i input=-
WARNING: Invalid field separator, using 'p'
WARNING: Invalid field separator, using 'p'
-4813902.92p-9497864.79p0.00
}}}

G_OPT_F_SEP is used in r.out.xyz, v.in.lines, r.tileset and m.proj.

r.out.xyz shouldn't be a problem, as the option is simply being passed
through to r.stats.

The other three all use the option value.

v.in.lines and m.proj explicitly understand space, tab and comma (although
I'm not sure that they interpret tab correctly; they appear to treat it as
a synonym for space).

r.tileset doesn't understand any of the names, interpreting the value
literally.

Also, none of this changes the fact that there appears to be a bug
somewhere. There '''shouldn't''' be any problems with using a literal
vertical bar character. If there are such problems, they should be fixed,
not simply worked around so that we can pretend there isn't a problem.

While there are valid reasons for supporting "separator=pipe" (e.g. not
forcing users to understand how quoting/escaping works in their preferred
shell), if it results in the original problem being forgotten about, it
may need to be reverted until the problem is fixed.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2314#comment:16>
GRASS GIS <http://grass.osgeo.org>

GDAL · May 31, 2014, 8:58am

#2314: output r.out.xyz
-------------------------------------------------+--------------------------
Reporter: pvanbosgeo | Owner: grass-dev@…
Type: defect | Status: new
Priority: normal | Milestone: 7.0.0
Component: Default | Version: svn-trunk
Keywords: separator, pipe, r.out.xyz, r.stats | Platform: MSWindows 7
Cpu: All |
-------------------------------------------------+--------------------------

Comment(by hcho):

Replying to [comment:16 glynn]:
> It adds a few new ones, namely that Python scripts (which don't use
G_option_to_separator()) are now trying to use the literal string "pipe"
as a separator. E.g.
> {{{
> $ echo "170.510125 -45.868537" | m.proj -i input=-
> WARNING: Invalid field separator, using 'p'
> WARNING: Invalid field separator, using 'p'
> -4813902.92p-9497864.79p0.00
> }}}
>
> G_OPT_F_SEP is used in r.out.xyz, v.in.lines, r.tileset and m.proj.
>
> r.out.xyz shouldn't be a problem, as the option is simply being passed
through to r.stats.
>
> The other three all use the option value.
>
> v.in.lines and m.proj explicitly understand space, tab and comma
(although I'm not sure that they interpret tab correctly; they appear to
treat it as a synonym for space).
>
> r.tileset doesn't understand any of the names, interpreting the value
literally.
>

I overlooked that some scripts use answers literally. Their default answer
just happened to be "|", not "comma", or some other strings, so we didn't
notice this issue so far.

> Also, none of this changes the fact that there appears to be a bug
somewhere. There '''shouldn't''' be any problems with using a literal
vertical bar character. If there are such problems, they should be fixed,
not simply worked around so that we can pretend there isn't a problem.
>

I agree if it can be fixed. E.g., the user can use "|", not "pipe" in the
wxGUI console. The backtick issue was worked around because it couldn't be
fixed (?) in Windows.

> While there are valid reasons for supporting "separator=pipe" (e.g. not
forcing users to understand how quoting/escaping works in their preferred
shell),

That very reason was why I chose to add "pipe" and as a side effect, this
issue was partially "taken care of".

> if it results in the original problem being forgotten about, it may need
to be reverted until the problem is fixed.

What about implementing grass.option_to_separator and keeping this ticket
open until the original issue is fixed?

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2314#comment:17>
GRASS GIS <http://grass.osgeo.org>

GDAL · May 31, 2014, 11:19am

#2314: output r.out.xyz
-------------------------------------------------+--------------------------
Reporter: pvanbosgeo | Owner: grass-dev@…
Type: defect | Status: new
Priority: normal | Milestone: 7.0.0
Component: Default | Version: svn-trunk
Keywords: separator, pipe, r.out.xyz, r.stats | Platform: MSWindows 7
Cpu: All |
-------------------------------------------------+--------------------------

Comment(by annakrat):

Replying to [comment:15 glynn]:
> Replying to [comment:12 annakrat]:
>
> > It's not problem of wxGUI but maybe of Python scripting library.
Today's version of r.out.xyz on Windows gives this error when you specify
| or you leave default:
> >
  {{{
  The syntax of the command is incorrect.
  }}}
>
> How are you running the script?

Sorry I forgot to mention it, I tried to run it from wxGUI and terminal
window and the error is the same. So I repeat it again, it's not the
problem of wxGUI.

>
> If a command is executed via e.g. grass.run_command(), Python's
subprocess.Popen() provides the necessary quoting.

Apparently not.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2314#comment:18>
GRASS GIS <http://grass.osgeo.org>

Huidae_Cho · May 31, 2014, 2:18pm

Check r60634. ^|& had to be escaped with ^^^.

···

On Sat, May 31, 2014 at 7:19 AM, GRASS GIS <trac@osgeo.org> wrote:

#2314: output r.out.xyz
-------------------------------------------------±-------------------------
Reporter: pvanbosgeo | Owner: grass-dev@…
Type: defect | Status: new
Priority: normal | Milestone: 7.0.0
Component: Default | Version: svn-trunk
Keywords: separator, pipe, r.out.xyz, r.stats | Platform: MSWindows 7
Cpu: All |
-------------------------------------------------±-------------------------

Comment(by annakrat):

Replying to [comment:15 glynn]:

Replying to [comment:12 annakrat]:

It’s not problem of wxGUI but maybe of Python scripting library.
Today’s version of r.out.xyz on Windows gives this error when you specify
| or you leave default:

{{{
The syntax of the command is incorrect.
}}}

How are you running the script?

Sorry I forgot to mention it, I tried to run it from wxGUI and terminal
window and the error is the same. So I repeat it again, it’s not the
problem of wxGUI.

If a command is executed via e.g. grass.run_command(), Python’s
subprocess.Popen() provides the necessary quoting.

Apparently not.

–
Ticket URL: <http://trac.osgeo.org/grass/ticket/2314#comment:18>
GRASS GIS <http://grass.osgeo.org>