[GRASS-dev] [GRASS GIS] #3010: PyGRASS fails to write vector map with attributes

#3010: PyGRASS fails to write vector map with attributes
----------------------+-------------------------
Reporter: annakrat | Owner: grass-dev@…
     Type: defect | Status: new
Priority: major | Milestone: 7.1.0
Component: PyGRASS | Version: svn-trunk
Keywords: vector | CPU: Unspecified
Platform: All |
----------------------+-------------------------
Example taken from a workshop material:

{{{
from grass.pygrass.vector import VectorTopo
from grass.pygrass.vector.geometry import Point
point1 = Point(635818.8, 221342.4)
point2 = Point(633627.7, 227050.2)
cols = [(u'cat', 'INTEGER PRIMARY KEY'),
         (u'name', 'TEXT')]

with VectorTopo('my_points', mode='w', tab_cols=cols, overwrite=True) as
my_points:
     # save the point and the attribute
     my_points.write(point1, ('pub', ))
     my_points.write(point2, ('restaurant', ))
     # save the changes to the database
     my_points.table.conn.commit()
}}}

gives:

{{{
WARNING: Vector map <my_points> already exists and will be overwritten
WARNING: Coor file of vector map <my_points@PERMANENT> is larger than it
          should be (18 bytes excess)
Building topology for vector map <my_points@PERMANENT>...
Registering primitives...
0 primitives registered
0 vertices registered
Building areas...
  100%
0 areas built
0 isles built
Attaching islands...
Attaching centroids...
Number of nodes: 0
Number of primitives: 0
Number of points: 0
Number of lines: 0
Number of boundaries: 0
Number of centroids: 0
Number of areas: 0
Number of isles: 0
Traceback (most recent call last):
   File "<stdin>", line 3, in <module>
   File "/home/anna/dev/grass/trunk1/dist.x86_64-pc-linux-
gnu/etc/python/grass/pygrass/errors.py", line 15, in wrapper
     return method(self, *args, **kargs)
   File "/home/anna/dev/grass/trunk1/dist.x86_64-pc-linux-
gnu/etc/python/grass/pygrass/vector/__init__.py", line 197, in write
     cats.set(cat, self.layer)
   File "/home/anna/dev/grass/trunk1/dist.x86_64-pc-linux-
gnu/etc/python/grass/pygrass/vector/basic.py", line 452, in set
     libvect.Vect_cat_set(self.c_cats, layer, cat)
ctypes.ArgumentError: argument 3: <type 'exceptions.TypeError'>: wrong
type
}}}

Works in 7.0.4.

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

#3010: PyGRASS fails to write vector map with attributes
--------------------------+-------------------------
  Reporter: annakrat | Owner: grass-dev@…
      Type: defect | Status: closed
  Priority: major | Milestone: 7.2.0
Component: PyGRASS | Version: svn-trunk
Resolution: fixed | Keywords: vector
       CPU: Unspecified | Platform: All
--------------------------+-------------------------
Changes (by annakrat):

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

Comment:

OK, the API changed in r66016 and the documentation was not updated. Fixed
in r68603 and backported in r68604 to releasebranch 7.2.

The right syntax is now:
{{{
new.write(point0, cat=1, attrs=('pub',))
}}}

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

#3010: PyGRASS fails to write vector map with attributes
-------------------------+-------------------------------------------------
  Reporter: annakrat | Owner: grass-dev@…
      Type: defect | Status: reopened
  Priority: major | Milestone: 7.2.0
Component: PyGRASS | Version: svn-trunk
Resolution: | Keywords: vector, API break, backwards
       CPU: | compatibility
  Unspecified | Platform: All
-------------------------+-------------------------------------------------
Changes (by wenzeslaus):

* keywords: vector => vector, API break, backwards compatibility
* status: closed => reopened
* resolution: fixed =>

Comment:

The following from r68603 is breaking backwards compatibility between 7.0
and 7.2:

{{{
#!diff
- def write(self, geo_obj, attrs=None, set_cats=True):
+ def write(self, geo_obj, cat=None, attrs=None):
}}}

The old API should (must) be accommodated. If the old API is so bad, that
it needs to be replaced (rather than extended), then we should have one
transitional version where both APIs work and the old one gives a warning.
I don't have enough information to make the judgment.

Related to that, the error message does not give much information about
what is going on.

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

#3010: PyGRASS fails to write vector map with attributes
-------------------------+-------------------------------------------------
  Reporter: annakrat | Owner: grass-dev@…
      Type: defect | Status: reopened
  Priority: major | Milestone: 7.2.0
Component: PyGRASS | Version: svn-trunk
Resolution: | Keywords: vector, API break, backwards
       CPU: | compatibility
  Unspecified | Platform: All
-------------------------+-------------------------------------------------

Comment (by martinl):

Replying to [comment:3 wenzeslaus]:
> The following from r68603 is breaking backwards compatibility between
7.0 and 7.2:

There are much more breakage of backward compatibility between 7.0 and 7.2
(speaking about pyGRASS). This is just one of them.

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

#3010: PyGRASS fails to write vector map with attributes
-------------------------+-------------------------------------------------
  Reporter: annakrat | Owner: grass-dev@…
      Type: defect | Status: reopened
  Priority: major | Milestone: 7.2.0
Component: PyGRASS | Version: svn-trunk
Resolution: | Keywords: vector, API break, backwards
       CPU: | compatibility
  Unspecified | Platform: All
-------------------------+-------------------------------------------------

Comment (by huhabla):

IMHO the former implementation was close to be a bug that was fixed in
trunk. For every new feature a new category was created by default, except
a category was already attached to the c_cat field in the geometry. It was
not obvious howto use the same category for different features.

Replying to [comment:3 wenzeslaus]:
> The following from r68603 is breaking backwards compatibility between
7.0 and 7.2:
>
> {{{
> #!diff
> - def write(self, geo_obj, attrs=None, set_cats=True):
> + def write(self, geo_obj, cat=None, attrs=None):
> }}}
>
> The old API should (must) be accommodated. If the old API is so bad,
that it needs to be replaced (rather than extended), then we should have
one transitional version where both APIs work and the old one gives a
warning. I don't have enough information to make the judgment.
>
> Related to that, the error message does not give much information about
what is going on.

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

#3010: PyGRASS fails to write vector map with attributes
-------------------------+-------------------------------------------------
  Reporter: annakrat | Owner: grass-dev@…
      Type: defect | Status: reopened
  Priority: major | Milestone: 7.2.0
Component: PyGRASS | Version: svn-trunk
Resolution: | Keywords: vector, API break, backwards
       CPU: | compatibility
  Unspecified | Platform: All
-------------------------+-------------------------------------------------

Comment (by zarch):

Replying to [comment:5 huhabla]:
> IMHO the former implementation was close to be a bug that was fixed in
trunk. For every new feature a new category was created by default, except
a category was already attached to the c_cat field in the geometry. It was
not obvious howto use the same category for different features.

I do think that the current version is better than it was.

But it breaks things and this is why I've informally ask to Markus N. if
it would be possible to have a grass addons directory grass70 and
grass72... But he want to avoid to have further differentiation of the
grass folders.
Because the code is not portable between these two versions.

[off topic] Perhaps git would be an option where you use a dedicate branch
for grass6/grass70/grass7x.

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

#3010: PyGRASS fails to write vector map with attributes
-------------------------+-------------------------------------------------
  Reporter: annakrat | Owner: grass-dev@…
      Type: defect | Status: reopened
  Priority: major | Milestone: 7.2.0
Component: PyGRASS | Version: svn-trunk
Resolution: | Keywords: vector, API break, backwards
       CPU: | compatibility
  Unspecified | Platform: All
-------------------------+-------------------------------------------------

Comment (by wenzeslaus):

Replying to [comment:5 huhabla]:
> IMHO the former implementation was close to be a bug that was fixed in
trunk.

If it was buggy, than there should be a special `if` with an error message
(exception, fatal, assert) which will tell you that you are using old API
which was buggy and you need to change your code.

BTW, the current message is not that good even in context of the 7.2 API
only because you don't know what has the wrong type.

> For every new feature a new category was created by default, except a
category was already attached to the c_cat field in the geometry.

This does not seem buggy but as a simple one which not flexible enough for
advanced user but pretty good for new users.

> It was not obvious howto use the same category for different features.

This sounds like a problem with documentation or a need for additional
API, seems like a long jump to breaking backwards compatibility.

Don't take me wrong, I agree that the new API is needed. It just needs
some better handling for the 7.0 API. (We are getting complains about
breaking API for modules between 6.4 and 7.0, so I suppose users care
about backwards compatibility.)

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

#3010: PyGRASS fails to write vector map with attributes
-------------------------+-------------------------------------------------
  Reporter: annakrat | Owner: grass-dev@…
      Type: defect | Status: reopened
  Priority: major | Milestone: 7.2.0
Component: PyGRASS | Version: svn-trunk
Resolution: | Keywords: vector, API break, backwards
       CPU: | compatibility
  Unspecified | Platform: All
-------------------------+-------------------------------------------------

Comment (by wenzeslaus):

In [changeset:"69809" 69809]:
{{{
#!CommitTicketReference repository="" revision="69809"
pygrass: 7.0 API now works even without using kwargs for Vector.write(),
see #3010
}}}

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

#3010: PyGRASS fails to write vector map with attributes
-------------------------+-------------------------------------------------
  Reporter: annakrat | Owner: grass-dev@…
      Type: defect | Status: reopened
  Priority: major | Milestone: 7.2.0
Component: PyGRASS | Version: svn-trunk
Resolution: | Keywords: vector, API break, backwards
       CPU: | compatibility
  Unspecified | Platform: All
-------------------------+-------------------------------------------------

Comment (by wenzeslaus):

Replying to [comment:4 martinl]:
> Replying to [comment:3 wenzeslaus]:
> > The following from r68603 is breaking backwards compatibility between
7.0 and 7.2:
>
> There are much more breakage of backward compatibility between 7.0 and
7.2 (speaking about pyGRASS). This is just one of them.

If there is more, it would be good to have them in
wiki:Release/7.2.0-News#PyGRASSchanges.

For the parts where the feature is a basic feature (like writing
attributes) or it is easy to implement (like switching parameters) we
should keep compatibility like in r69809 (the cat auto-incrementing was
already there).

How to report that to the user is a question. I used

{{{
#!python
import warnings
warnings.warn("Vector.write(geo_obj, attrs=(...)) is"
               " depreciated, specify cat explicitly",
               DeprecationWarning)
}}}

which is a more conservative option - by default it is visible only with
`python -Qwarn`.

If OK, r69809 needs backport to 7.2.

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

#3010: PyGRASS fails to write vector map with attributes
-------------------------+-------------------------------------------------
  Reporter: annakrat | Owner: grass-dev@…
      Type: defect | Status: closed
  Priority: major | Milestone: 7.2.0
Component: PyGRASS | Version: svn-trunk
Resolution: fixed | Keywords: vector, API break, backwards
       CPU: | compatibility
  Unspecified | Platform: All
-------------------------+-------------------------------------------------
Changes (by wenzeslaus):

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

Comment:

In [changeset:"69832" 69832]:
{{{
#!CommitTicketReference repository="" revision="69832"
pygrass: 7.0 API now works even without using kwargs for Vector.write(),
closes #3010
}}}

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