[GRASS-dev] [GRASS GIS] #3546: t.rast.what: issues in one_point_per_col_output()

#3546: t.rast.what: issues in one_point_per_col_output()
-------------------------+-------------------------
Reporter: sbl | Owner: grass-dev@…
     Type: defect | Status: new
Priority: normal | Milestone: 7.4.1
Component: Temporal | Version: 7.4.0
Keywords: t.rast.what | CPU: All
Platform: All |
-------------------------+-------------------------
There is a ; hardcoded in the header for one_point_per_col_output() in
t.rast.what, which causes columns to shift.
In addition, there is an indentation error.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3546&gt;
GRASS GIS <https://grass.osgeo.org>

#3546: t.rast.what: issues in one_point_per_col_output()
-----------------------+-------------------------
  Reporter: sbl | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.4.1
Component: Temporal | Version: 7.4.0
Resolution: | Keywords: t.rast.what
       CPU: All | Platform: All
-----------------------+-------------------------

Comment (by sbl):

In [changeset:"72635" 72635]:
{{{
#!CommitTicketReference repository="" revision="72635"
fix header and indentation for one_point_per_col_output; see #3546
}}}

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3546#comment:1&gt;
GRASS GIS <https://grass.osgeo.org>

#3546: t.rast.what: issues in one_point_per_col_output()
-----------------------+-------------------------
  Reporter: sbl | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.4.1
Component: Temporal | Version: 7.4.0
Resolution: | Keywords: t.rast.what
       CPU: All | Platform: All
-----------------------+-------------------------

Comment (by sbl):

Please test the (tiny) fix in trunk before backporting.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3546#comment:2&gt;
GRASS GIS <https://grass.osgeo.org>

#3546: t.rast.what: issues in one_point_per_col_output()
-----------------------+-------------------------
  Reporter: sbl | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.4.1
Component: Temporal | Version: 7.4.0
Resolution: | Keywords: t.rast.what
       CPU: All | Platform: All
-----------------------+-------------------------

Comment (by sbl):

In [changeset:"72636" 72636]:
{{{
#!CommitTicketReference repository="" revision="72636"
use different separator for coordinates and sites; see #3546
}}}

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3546#comment:3&gt;
GRASS GIS <https://grass.osgeo.org>

#3546: t.rast.what: issues in one_point_per_col_output()
-----------------------+-------------------------
  Reporter: sbl | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.4.1
Component: Temporal | Version: 7.4.0
Resolution: | Keywords: t.rast.what
       CPU: All | Platform: All
-----------------------+-------------------------

Comment (by sbl):

Sorry, that first fix was not working properly. r72636 is more complete
and works as expected here.

However, additional documentation might be needed!? Users should be
probably informed that default separator for coordinates (and site) is
comma, if comma is not the main column separator. In the latter case
semicolon is used as spearator for coordinates and site.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3546#comment:4&gt;
GRASS GIS <https://grass.osgeo.org>

#3546: t.rast.what: issues in one_point_per_col_output()
-----------------------+-------------------------
  Reporter: sbl | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.4.1
Component: Temporal | Version: 7.4.0
Resolution: | Keywords: t.rast.what
       CPU: All | Platform: All
-----------------------+-------------------------

Comment (by sbl):

Can be tested with example from the
[https://grass.osgeo.org/grass74/manuals/t.rast.what.html manual]:

{{{
# Create test data
g.region s=0 n=80 w=0 e=120 b=0 t=50 res=10

# Generate data
r.mapcalc expression="a_1 = 1" -s
r.mapcalc expression="a_2 = 2" -s
r.mapcalc expression="a_3 = 3" -s
r.mapcalc expression="a_4 = 4" -s

t.create type=strds output=A title="A test" descr="A test"

t.register -i type=raster input=A maps=a_1,a_2,a_3,a_4 \
     start='1990-01-01' increment="1 month"

v.random output=points n=3 seed=1
}}}

{{{
#Query STRDS (please try this with and without r72636
t.rast.what strds=A points=points layout=col -nv
}}}

Pay attention to the changes in the separators for the coordinates and cat
in the header (and the different number of resulting columns)...

Another option could be to remove coordinates from output, when cat is
used (v-flag), cause coordinates would be redundant.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3546#comment:5&gt;
GRASS GIS <https://grass.osgeo.org>

#3546: t.rast.what: issues in one_point_per_col_output()
-----------------------+-------------------------
  Reporter: sbl | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.4.1
Component: Temporal | Version: 7.4.0
Resolution: | Keywords: t.rast.what
       CPU: All | Platform: All
-----------------------+-------------------------

Comment (by veroandreo):

I tested the fix following the example in trunk (with the fix) and in
release branch. Personally, I would prefer to get only cat number when
using -v flag. Default could still be the coords (-n) and -v only cat
values.

Moreover, testing different layouts, I found that when using
`layout=timerow`, there's an extra empty column or line according if I use
-vn or only -v (but this also happens in release branch, without the fix):

{{{
t.rast.what strds=A points=points layout=timerow -vn
cat|x|y|1990-01-01 00:00:00;1990-02-01 00:00:00|1990-02-01
00:00:00;1990-03-01 00:00:00|1990-03-01 00:00:00;1990-04-01
00:00:00|1990-04-01 00:00:00;1990-05-01 00:00:00
1|115.004358627375|36.3593955782903||1|2|3|4
2|79.681676382576|45.2391522852909||1|2|3|4
3|97.4892579600048|79.2347263950131||1|2|3|4

t.rast.what strds=A points=points layout=timerow -v

1|115.004358627375|36.3593955782903||1|2|3|4
2|79.681676382576|45.2391522852909||1|2|3|4
3|97.4892579600048|79.2347263950131||1|2|3|4
}}}

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3546#comment:6&gt;
GRASS GIS <https://grass.osgeo.org>

#3546: t.rast.what: issues in one_point_per_col_output()
-----------------------+-------------------------
  Reporter: sbl | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.4.1
Component: Temporal | Version: 7.4.0
Resolution: | Keywords: t.rast.what
       CPU: All | Platform: All
-----------------------+-------------------------
Changes (by sbl):

* Attachment "t.rast.what_timerow.diff" added.

Fix empty column (and header) in timerow outout

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3546&gt;
GRASS GIS <https://grass.osgeo.org>

#3546: t.rast.what: issues in one_point_per_col_output()
-----------------------+-------------------------
  Reporter: sbl | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.4.1
Component: Temporal | Version: 7.4.0
Resolution: | Keywords: t.rast.what
       CPU: All | Platform: All
-----------------------+-------------------------

Comment (by sbl):

Thanks for testing! If there is agreement on removing coordinates if cat
is used, I can change that. Not sure if that can go to 7.4, though???

Please also test attached new diff that fixes the empty column in the
timerow output and an empty line if header is not requested...

Looks now like this:

{{{
t.rast.what strds=A points=points layout=timerow -v
1|115.004358627375|36.3593955782903|1|2|3|4
2|79.681676382576|45.2391522852909|1|2|3|4
3|97.4892579600048|79.2347263950131|1|2|3|4

t.rast.what strds=A points=points layout=timerow -vn
cat|x|y|1990-01-01 00:00:00;1990-02-01 00:00:00|1990-02-01
00:00:00;1990-03-01 00:00:00|1990-03-01 00:00:00;1990-04-01
00:00:00|1990-04-01 00:00:00;1990-05-01 00:00:00
1|115.004358627375|36.3593955782903|1|2|3|4
2|79.681676382576|45.2391522852909|1|2|3|4
3|97.4892579600048|79.2347263950131|1|2|3|4

t.rast.what strds=A points=points layout=timerow
115.004358627375|36.3593955782903|1|2|3|4
79.681676382576|45.2391522852909|1|2|3|4
97.4892579600048|79.2347263950131|1|2|3|4

echo "115 36 Loc1
79 45 Loc2" | t.rast.what -i strds=A layout=timerow -n separator=','
x,y,site,1990-01-01 00:00:00;1990-02-01 00:00:00,1990-02-01
00:00:00;1990-03-01 00:00:00,1990-03-01 00:00:00;1990-04-01
00:00:00,1990-04-01 00:00:00;1990-05-01 00:00:00
115,36,Loc1,1,2,3,4
79,45,Loc2,1,2,3,4
}}}

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3546#comment:7&gt;
GRASS GIS <https://grass.osgeo.org>

#3546: t.rast.what: issues in one_point_per_col_output()
-----------------------+-------------------------
  Reporter: sbl | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.4.1
Component: Temporal | Version: 7.4.0
Resolution: | Keywords: t.rast.what
       CPU: All | Platform: All
-----------------------+-------------------------

Comment (by veroandreo):

Replying to [comment:7 sbl]:
> Thanks for testing! If there is agreement on removing coordinates if cat
is used, I can change that. Not sure if that can go to 7.4, though???

IMHO, it's a minor change in the behavior of a flag, but I don't think it
can go into 7.4. I guess into 7.6, no?

> Please also test attached new diff that fixes the empty column in the
timerow output and an empty line if header is not requested...

Just tested the patch, looks exactly as you showed, great :slight_smile:

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3546#comment:8&gt;
GRASS GIS <https://grass.osgeo.org>

#3546: t.rast.what: issues in one_point_per_col_output()
-----------------------+-------------------------
  Reporter: sbl | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.4.1
Component: Temporal | Version: 7.4.0
Resolution: | Keywords: t.rast.what
       CPU: All | Platform: All
-----------------------+-------------------------

Comment (by sbl):

In [changeset:"72713" 72713]:
{{{
#!CommitTicketReference repository="" revision="72713"
remove empty header and column in timerow output; see #3546
}}}

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3546#comment:9&gt;
GRASS GIS <https://grass.osgeo.org>

#3546: t.rast.what: issues in one_point_per_col_output()
-----------------------+-------------------------
  Reporter: sbl | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.4.1
Component: Temporal | Version: 7.4.0
Resolution: | Keywords: t.rast.what
       CPU: All | Platform: All
-----------------------+-------------------------

Comment (by sbl):

OK, then I keep removing coordinates if cat is used separate from this
ticket.
If there are no objections, I would like to backport changes above, in
order to get properly aligned output in 7.4.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3546#comment:10&gt;
GRASS GIS <https://grass.osgeo.org>

#3546: t.rast.what: issues in one_point_per_col_output()
-----------------------+-------------------------
  Reporter: sbl | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.4.1
Component: Temporal | Version: 7.4.0
Resolution: | Keywords: t.rast.what
       CPU: All | Platform: All
-----------------------+-------------------------

Comment (by sbl):

BTW, 4 unit tests are failing in current release branch:

{{{
FAIL: test_col_output (__main__.TestRasterWhat)
----------------------------------------------------------------------
(...)
AssertionError: File <out_col.txt> does not have the right MD5 sum.
Expected is <8720cc237b46ddb18f11440469d0e13f>, actual is
<885b6f50405b08fa9fe9ae33ed50e29b>

======================================================================
FAIL: test_col_output_cat (__main__.TestRasterWhat)
----------------------------------------------------------------------
(...)
AssertionError: File <out_col_cat.txt> does not have the right MD5 sum.
Expected is <e1d8e6651b3bff1c35e366e48d634db4>, actual is
<ac0a9b14e59920c3f8b5834282a24822>

======================================================================
FAIL: test_col_output_coords (__main__.TestRasterWhat)
----------------------------------------------------------------------
(...)
AssertionError: File <out_col_coords.txt> does not have the right MD5 sum.
Expected is <ac44ebc5aa040d4ce2a5787e95f208ec>, actual is
<ecdc79a6880a9e1f163cc92fa384b8a3>

======================================================================
FAIL: test_timerow_output_cat (__main__.TestRasterWhat)
----------------------------------------------------------------------
(...)
AssertionError: File <out_col_trow.txt> does not have the right MD5 sum.
Expected is <55e2ff8ddaeb731a73daca48adf2768d>, actual is
<1da3350f488df9c919b4009625956b3b>
}}}

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3546#comment:11&gt;
GRASS GIS <https://grass.osgeo.org>

#3546: t.rast.what: issues in one_point_per_col_output()
-----------------------+-------------------------
  Reporter: sbl | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.4.1
Component: Temporal | Version: 7.4.0
Resolution: | Keywords: t.rast.what
       CPU: All | Platform: All
-----------------------+-------------------------

Comment (by sbl):

In [changeset:"72716" 72716]:
{{{
#!CommitTicketReference repository="" revision="72716"
Make tests pass again; see #3546
}}}

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3546#comment:12&gt;
GRASS GIS <https://grass.osgeo.org>

#3546: t.rast.what: issues in one_point_per_col_output()
-----------------------+-------------------------
  Reporter: sbl | Owner: grass-dev@…
      Type: defect | Status: closed
  Priority: normal | Milestone: 7.4.1
Component: Temporal | Version: 7.4.0
Resolution: fixed | Keywords: t.rast.what
       CPU: All | Platform: All
-----------------------+-------------------------
Changes (by sbl):

* status: new => closed
* resolution: => fixed

Comment:

In [changeset:"72717" 72717]:
{{{
#!CommitTicketReference repository="" revision="72717"
Correct output format (trunk, r72635, r72636, r72713, r72716); fix #3546
}}}

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3546#comment:13&gt;
GRASS GIS <https://grass.osgeo.org>