initial support for geopackage metatata #715

snowman2 · 2019-01-28T15:32:06Z

issue #713

@sgillies here is the first iteration.

snowman2 · 2019-01-28T15:45:18Z

Strange, it says here that the metadata has been supported since GDAL 1.11, but that seems to be the only one failing.

snowman2 · 2019-01-28T15:47:59Z

Hmmm, any ideas on this error:

E   fiona.errors.DriverError: sqlite3_open(/vsimem//tmp/pytest-of-travis/pytest-0/test_set_tag_item__namespace_N0/test.gpkg) failed: out of memory

coveralls · 2019-01-28T16:00:51Z

Coverage increased (+0.04%) to 83.147% when pulling 844d77b on snowman2:issue_713 into 5a67508 on Toblerity:master.

coveralls · 2019-01-28T16:00:53Z

Coverage decreased (-0.4%) to 82.2% when pulling 407674b on snowman2:issue_713 into cd5da17 on Toblerity:master.

coveralls · 2019-01-28T16:00:55Z

Coverage decreased (-0.4%) to 82.2% when pulling 407674b on snowman2:issue_713 into cd5da17 on Toblerity:master.

coveralls · 2019-01-28T16:00:56Z

Coverage decreased (-0.4%) to 82.2% when pulling 407674b on snowman2:issue_713 into cd5da17 on Toblerity:master.

snowman2 · 2019-01-28T16:22:32Z

GDAL 1.11 is the only one that fails to get/set metadata correctly. How would you like that handled?

fiona/ogrext.pyx

fiona/collection.py

snorfalorpagus · 2019-01-28T23:11:27Z

Despite the documentation it seems to me that GDAL 1.x doesn't support GPKG metadata:

>>> from osgeo import ogr
>>> driver = ogr.GetDriverByName("GPKG")
>>> ds = driver.CreateDataSource("new.gpkg")
>>> ds.SetMetadataItem("foo", "bar")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python3/dist-packages/osgeo/ogr.py", line 577, in <lambda>
    __getattr__ = lambda self, name: _swig_getattr(self, DataSource, name)
  File "/usr/lib/python3/dist-packages/osgeo/ogr.py", line 74, in _swig_getattr
    return _swig_getattr_nondynamic(self, class_type, name, 0)
  File "/usr/lib/python3/dist-packages/osgeo/ogr.py", line 69, in _swig_getattr_nondynamic
    return object.__getattr__(self, name)
AttributeError: type object 'object' has no attribute '__getattr__'

snowman2 · 2019-01-29T01:47:38Z

Despite the documentation it seems to me that GDAL 1.x doesn't support GPKG metadata:

That's unfortunate. Maybe adding the decorator require_gdal_version to require GDAL 2.x would be the solution here?

snorfalorpagus · 2019-01-29T21:30:14Z

fiona/env.py

@@ -538,7 +538,6 @@ def some_func(foo=None):
    reason = '\n{0}'.format(reason) if reason else reason

    def decorator(f):
-        @wraps(f)


It seems a shame to make the experience on Python 3 worse in order to support Python 2, which is EOL this year.

We don't have a formal plan for what we're doing with Python 2 yet. Thoughts @sgillies?

wraps wasn't breaking anything before. Can you explain what has changed @snowman2 ?

The difference here is that it is being used on a class method instead of a function. It works fine for functions. However, when being applied to a class method, it breaks in Python 2 see build logs This issue was resolved in Python 3.2 see here. Definitely open to ideas for a better solution.

sgillies · 2019-03-04T23:27:07Z

I've finally made some time to get to this! Here's a test that was failing before the require_gdal_version decorator was added.

_________________________ test_set_tag_item[test-test] _________________________
layer = 'test', namespace = 'test'
tmpdir = local('/tmp/pytest-of-travis/pytest-0/test_set_tag_item_test_test_0')
    @pytest.mark.parametrize("layer, namespace", [
        (None, None),
        (None, "test"),
        ("test", None),
        ("test", "test"),
    ])
    def test_set_tag_item(layer, namespace, tmpdir):
        test_geopackage = str(tmpdir.join("test.gpkg"))
        schema = {'properties': {'CDATA1': 'str:254'}, 'geometry': 'Polygon'}
        with fiona.open(test_geopackage, "w", driver="GPKG", schema=schema, layer=layer) as gpkg:
            assert gpkg.get_tag_item("test_tag1", ns=namespace) is None
            gpkg.set_tag_item("test_tag1", "test_value1", ns=namespace)
    
        with fiona.open(test_geopackage, layer=layer) as gpkg:
            if namespace is not None:
                assert gpkg.get_tag_item("test_tag1") is None
>           assert gpkg.get_tag_item("test_tag1", ns=namespace) == "test_value1"
E           AssertionError: assert None == 'test_value1'
E            +  where None = <bound method Collection.get_tag_item of <open Collection '/tmp/pytest-of-travis/pytest-0/test_set_tag_item_test_test_0/test.gpkg:test', mode 'r' at 0x7fabc2bb87b8>>('test_tag1', ns='test')
E            +    where <bound method Collection.get_tag_item of <open Collection '/tmp/pytest-of-travis/pytest-0/test_set_tag_item_test_test_0/test.gpkg:test', mode 'r' at 0x7fabc2bb87b8>> = <open Collection '/tmp/pytest-of-travis/pytest-0/test_set_tag_item_test_test_0/test.gpkg:test', mode 'r' at 0x7fabc2bb87b8>.get_tag_item
tests/test_session.py:54: AssertionError
----------------------------- Captured stderr call -----------------------------
ERROR 1: sqlite3_prepare_v2(SELECT Count(*) FROM test WHERE test_tag1) failed: no such column: test_tag1
ERROR 1: failed to prepare SQL: SELECT fid, "geom", "CDATA1" FROM test WHERE test_tag1

The method returns None. If we had an Env block around the calls, we would get an exception from ogrext.pyx. Instead, the errors are printed to stderr.

I think it would be fine to remove the decorator and let this method raise an exception with GDAL 1.11. We'll document the situation and will be leaving 1.x behind sooner or later. It shouldn't block us from moving ahead.

@snorfalorpagus @snowman2 Yes? No?

snowman2 · 2019-03-05T00:43:48Z

Sounds like a good plan to me.

snowman2 · 2019-03-05T02:01:13Z

@sgillies I removed the commits with the decorators.

snowman2 · 2019-03-05T02:02:11Z

Should I add the env block in the tests?

sgillies · 2019-03-05T02:10:45Z

@snowman2 yes, please. And, if you like: catch the exception coming from fiona.ogrext with GDAL 1.11 and xfail the test inside the except clause using pytest.xfail.

snowman2 · 2019-03-06T01:32:01Z

xfail coming after I the next tests fail so I have the specific exception it raises.

snowman2 · 2019-03-06T01:38:50Z

@sgillies, it does not seem to raise an exception. How would you like to proceed?

sgillies · 2019-03-06T22:11:24Z

@snowman2 I'm surprised at the lack of an exception, but for now, let's test the dict and xfail if it is empty and if the GDAL version is less than 2.0.

sgillies · 2019-03-07T19:39:22Z

Congratulations @snowman2 ! I'm going to give this a little more review on the weekend, focusing on what to return or raise when the collection isn't open (no session or whatever). Generally, this is great stuff.

snowman2 · 2019-03-07T19:50:17Z

Just squashed it. 👍

* Add model module and tests model.Object is the dict with deprecation warnings class. * Reformat, give up on dict instance checks

joehuanguf · 2019-07-02T20:12:11Z

Is there still interest in this PR? I can take a stab at continuing it.

sgillies · 2019-07-12T02:47:07Z

@joehuanguf yes, this is just held up by a refactor and my involvement in GDAL 3 fallout. We're still going to do this.

* Add shim module and support for GDAL 3 * Add GDAL 2.4 and 3.0 to build matrix * Don't upgrade deps * Don't use setup.py directly * Add PROJ to the build matrix * Add proj build script * Update GDAL build script * Allow travis to wait for GDAL build * Wait 30 on GDAL build * Increase wait to 40 and make jobs to 4 * Remove wait so we can see what the problem is * Unsilence make * Remove proj apt package * set PROJ_LIB correctly * Call OSRFixup for GDAL 1 and 2 * Cast open options for GDALOpenEx * Add missing OSRFixup declarations

Toblerity#802)

sgillies · 2019-11-05T18:15:16Z

fiona/collection.py

+        """
+        if isinstance(self.session, WritingSession):
+            return self.session.set_tags(tags, ns=ns)
+        raise RuntimeError("Unable to set tags as not in writing mode.")


@snowman2 here I think we should begin with

if not isinstance(self.session, WritingSession): raise ...

and then I think we should consider a custom error. If we open a Python (3) file to read and then call its write method we get an io.UnsupportedOperation. We should add something like that to fiona.

Have a preference for the exception name?

sgillies

I have a couple changes to suggest. Also, would you be willing to re-target this at the maint-1.9 branch?

sgillies · 2019-11-05T18:17:37Z

Finally getting back to this. It's still relevant and good and I think it should be able to merge fairly cleanly with maint-1.9.

snowman2 · 2019-11-06T00:53:26Z

Didn't see dropdown to change branches. Moved to #821

snorfalorpagus reviewed Jan 28, 2019

View reviewed changes

fiona/ogrext.pyx Outdated Show resolved Hide resolved

snorfalorpagus reviewed Jan 28, 2019

View reviewed changes

fiona/collection.py Show resolved Hide resolved

snowman2 force-pushed the issue_713 branch from a7259a3 to 0df61fe Compare January 29, 2019 01:45

snowman2 force-pushed the issue_713 branch 2 times, most recently from 0230222 to 002daed Compare January 29, 2019 16:23

snorfalorpagus reviewed Jan 29, 2019

View reviewed changes

snorfalorpagus mentioned this pull request Jan 30, 2019

Support for Python 2.x #716

Closed

snowman2 force-pushed the issue_713 branch from 002daed to 8a0e477 Compare March 5, 2019 02:00

sgillies added the enhancement label Mar 7, 2019

sgillies added this to the 1.9 milestone Mar 7, 2019

snowman2 force-pushed the issue_713 branch from 828dc06 to 844d77b Compare March 7, 2019 19:49

sgillies added 2 commits June 9, 2019 20:01

Update version to 1.9dev

c097a81

Add model module and tests (Toblerity#759)

24755a2

* Add model module and tests model.Object is the dict with deprecation warnings class. * Reformat, give up on dict instance checks

Sean Gillies and others added 17 commits September 24, 2019 18:03

Merge tag '1.8.7' into maint-1.9

fe7d9cf

Merge tag '1.8.8' into maint-1.9

bf5d3a7

Update change log and version

5792747

Merge tag '1.8.9' of github.com:Toblerity/Fiona into maint-1.9

3039220

Merge branch 'maint-1.9' of github.com:Toblerity/Fiona into maint-1.9

cdd3542

use _shim3.c for gdal >3.0 (Toblerity#807)

8886a2a

Here comes 1.8.9.post1

c466307

Reformat version name

41436f8

Fix setup regression introduced in 1.8.9.post1

960568d

Update version and change log

fb97038

return None and raise warning when transform_geom dst_ogr_geom is NULL (

d0e94b5

Toblerity#802)

Merge branch 'maint-1.8' into maint-1.9

c81be54

Get MutableMapping from compat

d9378f6

Revert accidental change

12fd1c3

Skip test with GDAL 3

72e234d

modified transform geom to accept arrays of geoms (Toblerity#811)

a7f7af0

sgillies reviewed Nov 5, 2019

View reviewed changes

sgillies requested changes Nov 5, 2019

View reviewed changes

initial support for geopackage metatata issue#713

2af53d5

snowman2 force-pushed the issue_713 branch from 844d77b to 2af53d5 Compare November 6, 2019 00:50

snowman2 closed this Nov 6, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

initial support for geopackage metatata #715

initial support for geopackage metatata #715

snowman2 commented Jan 28, 2019

snowman2 commented Jan 28, 2019

snowman2 commented Jan 28, 2019

coveralls commented Jan 28, 2019 •

edited

Loading

coveralls commented Jan 28, 2019

coveralls commented Jan 28, 2019

coveralls commented Jan 28, 2019

snowman2 commented Jan 28, 2019

snorfalorpagus commented Jan 28, 2019

snowman2 commented Jan 29, 2019 •

edited

Loading

snorfalorpagus Jan 29, 2019

sgillies Jan 31, 2019

snowman2 Jan 31, 2019

sgillies commented Mar 4, 2019

snowman2 commented Mar 5, 2019

snowman2 commented Mar 5, 2019

snowman2 commented Mar 5, 2019

sgillies commented Mar 5, 2019

snowman2 commented Mar 6, 2019

snowman2 commented Mar 6, 2019

sgillies commented Mar 6, 2019

sgillies commented Mar 7, 2019

snowman2 commented Mar 7, 2019

joehuanguf commented Jul 2, 2019

sgillies commented Jul 12, 2019

sgillies Nov 5, 2019

snowman2 Nov 6, 2019

sgillies left a comment

sgillies commented Nov 5, 2019

snowman2 commented Nov 6, 2019

initial support for geopackage metatata #715

initial support for geopackage metatata #715

Conversation

snowman2 commented Jan 28, 2019

snowman2 commented Jan 28, 2019

snowman2 commented Jan 28, 2019

coveralls commented Jan 28, 2019 • edited Loading

coveralls commented Jan 28, 2019

coveralls commented Jan 28, 2019

coveralls commented Jan 28, 2019

snowman2 commented Jan 28, 2019

snorfalorpagus commented Jan 28, 2019

snowman2 commented Jan 29, 2019 • edited Loading

snorfalorpagus Jan 29, 2019

Choose a reason for hiding this comment

sgillies Jan 31, 2019

Choose a reason for hiding this comment

snowman2 Jan 31, 2019

Choose a reason for hiding this comment

sgillies commented Mar 4, 2019

snowman2 commented Mar 5, 2019

snowman2 commented Mar 5, 2019

snowman2 commented Mar 5, 2019

sgillies commented Mar 5, 2019

snowman2 commented Mar 6, 2019

snowman2 commented Mar 6, 2019

sgillies commented Mar 6, 2019

sgillies commented Mar 7, 2019

snowman2 commented Mar 7, 2019

joehuanguf commented Jul 2, 2019

sgillies commented Jul 12, 2019

sgillies Nov 5, 2019

Choose a reason for hiding this comment

snowman2 Nov 6, 2019

Choose a reason for hiding this comment

sgillies left a comment

Choose a reason for hiding this comment

sgillies commented Nov 5, 2019

snowman2 commented Nov 6, 2019

coveralls commented Jan 28, 2019 •

edited

Loading

snowman2 commented Jan 29, 2019 •

edited

Loading