PyGRASS fails to write vector map with attributes

[annakrat]

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.

[neteler]

Milestone renamed

[annakrat]

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',))

[wenzeslaus]

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

- 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.

[martinl]

Replying to 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.

[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 wenzeslaus:

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

- 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.

[zarch]

Replying to 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.

[wenzeslaus]

Replying to 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.)

[wenzeslaus]

In 69809:

pygrass: 7.0 API now works even without using kwargs for Vector.write(), see #3010

[wenzeslaus]

Replying to martinl:

Replying to 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

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.

[wenzeslaus]

In 69832:

pygrass: 7.0 API now works even without using kwargs for Vector.write(), closes #3010