From 201ae9cd93047db99cda9c683410b4bbceb9f1fc Mon Sep 17 00:00:00 2001 From: lmmarsano Date: Fri, 30 Nov 2018 00:05:08 -0500 Subject: [PATCH] BUG: fix from_postgis to accept nullable geometry (#856) * TST: test read_postgis with NULL geometry Though GIS databases permit NULL values in the geometry field, read_postgis throws exceptions when it encounters them. test_read_postgis_null_geom defines this test case with sqlite3 database, and support functions are revised to support it. create_sqlite is revised to loosen data type restrictions and follow [recommendations from the python manual](https://docs.python.org/3/library/sqlite3.html) - connection parameter instead of filename to allow for in-memory database - 'with' statement for transaction management - parameter substitution instead of string operations validate_boro_df strips NULL values before checking geometry type. * TST: replace sqlite test with spatialite test The original SQLite test reflected unlikely usage for geospatial analysis. In parity with PostGIS, the new test runs against SQLite with SpatiaLite enabled. For coverage, test geometries read as text and binary. * TST: add libspatialite as a test dependency * TST: add conda-forge testing environment for Python 2.7 add this environment to continuous integration conda-forge Python 2.7 enables loadable SQLite extensions with conda-forge/python-feedstock#227 adding this environment enables SpatiaLite tests * BUG: fix read_postgis & GeoDataFrame to accept NULL geometry values read_postgis failed to load nullable binary geometry data while GeoDataFrame raised exceptions on NULL in the geometry field. This fixes both issues. It moreover adds logic to detect python version & load geometries from python 2 buffers as necessary. * ENH DOC: drop hex_encoded parameter from read_postgis & from_postgis drop hex_encoded, since sqlite3 represents binary & text as distinct Python data types adjust names to clarify this distinction adjust documentation to clarify geometry is expected in WKB representation (which was implicit) add example SpatiaLite usage reflecting this expectation --- .travis.yml | 1 + ci/travis/27-dev.yaml | 1 + ci/travis/27-latest-conda-forge.yaml | 28 +++++++++ ci/travis/27-latest-defaults.yaml | 1 + ci/travis/27-pd020.yaml | 1 + ci/travis/35-minimal.yaml | 1 + ci/travis/36-pd020.yaml | 1 + ci/travis/36-pd022.yaml | 1 + ci/travis/37-dev.yaml | 1 + ci/travis/37-latest-conda-forge.yaml | 1 + ci/travis/37-latest-defaults.yaml | 1 + geopandas/geodataframe.py | 17 +++--- geopandas/io/sql.py | 51 +++++++++++------ geopandas/io/tests/test_io.py | 37 +++++++++++- geopandas/tests/util.py | 85 +++++++++++++++++++++------- 15 files changed, 178 insertions(+), 50 deletions(-) create mode 100644 ci/travis/27-latest-conda-forge.yaml diff --git a/.travis.yml b/.travis.yml index 6189d0a..262b7f5 100644 --- a/.travis.yml +++ b/.travis.yml @@ -10,6 +10,7 @@ matrix: # Python 2.7 and 3.6 test all supported Pandas versions - env: ENV_FILE="ci/travis/27-pd020.yaml" - env: ENV_FILE="ci/travis/27-latest-defaults.yaml" + - env: ENV_FILE="ci/travis/27-latest-conda-forge.yaml" - env: ENV_FILE="ci/travis/27-dev.yaml" - env: ENV_FILE="ci/travis/36-pd020.yaml" diff --git a/ci/travis/27-dev.yaml b/ci/travis/27-dev.yaml index 7c47c93..f00e969 100644 --- a/ci/travis/27-dev.yaml +++ b/ci/travis/27-dev.yaml @@ -23,6 +23,7 @@ dependencies: #- geopy - SQLalchemy - psycopg2 + - libspatialite - pip: - git+https://github.com/pydata/pandas.git - codecov diff --git a/ci/travis/27-latest-conda-forge.yaml b/ci/travis/27-latest-conda-forge.yaml new file mode 100644 index 0000000..5f9cb00 --- /dev/null +++ b/ci/travis/27-latest-conda-forge.yaml @@ -0,0 +1,28 @@ +name: test +channels: + - conda-forge +dependencies: + - python=2.7 + - six + # required + - pandas + - shapely + - fiona + - pyproj + # testing + - pytest + - pytest-cov + #- codecov + - mock + # optional + - rtree + - matplotlib + - descartes + - pysal + #- geopy + - SQLalchemy + - psycopg2 + - libspatialite + - pip: + - codecov + - geopy diff --git a/ci/travis/27-latest-defaults.yaml b/ci/travis/27-latest-defaults.yaml index 486d896..1888b16 100644 --- a/ci/travis/27-latest-defaults.yaml +++ b/ci/travis/27-latest-defaults.yaml @@ -22,6 +22,7 @@ dependencies: #- geopy - SQLalchemy - psycopg2 + - libspatialite - pip: - codecov - geopy diff --git a/ci/travis/27-pd020.yaml b/ci/travis/27-pd020.yaml index 4e75d91..929616c 100644 --- a/ci/travis/27-pd020.yaml +++ b/ci/travis/27-pd020.yaml @@ -24,3 +24,4 @@ dependencies: - geopy - SQLalchemy - psycopg2 + - libspatialite diff --git a/ci/travis/35-minimal.yaml b/ci/travis/35-minimal.yaml index 5f50d0b..c191c07 100644 --- a/ci/travis/35-minimal.yaml +++ b/ci/travis/35-minimal.yaml @@ -22,3 +22,4 @@ dependencies: - geopy - SQLalchemy - psycopg2 + - libspatialite diff --git a/ci/travis/36-pd020.yaml b/ci/travis/36-pd020.yaml index 120a8ea..7556ab2 100644 --- a/ci/travis/36-pd020.yaml +++ b/ci/travis/36-pd020.yaml @@ -21,3 +21,4 @@ dependencies: - geopy - SQLalchemy - psycopg2 + - libspatialite diff --git a/ci/travis/36-pd022.yaml b/ci/travis/36-pd022.yaml index d91a7a5..9adc938 100644 --- a/ci/travis/36-pd022.yaml +++ b/ci/travis/36-pd022.yaml @@ -21,6 +21,7 @@ dependencies: #- geopy - SQLalchemy - psycopg2 + - libspatialite - pip: - codecov - geopy diff --git a/ci/travis/37-dev.yaml b/ci/travis/37-dev.yaml index a0d71e2..727345d 100644 --- a/ci/travis/37-dev.yaml +++ b/ci/travis/37-dev.yaml @@ -22,6 +22,7 @@ dependencies: #- geopy - SQLalchemy - psycopg2 + - libspatialite - pip: - git+https://github.com/matplotlib/matplotlib.git - git+https://github.com/pydata/pandas.git diff --git a/ci/travis/37-latest-conda-forge.yaml b/ci/travis/37-latest-conda-forge.yaml index 8e6cb99..f40de82 100644 --- a/ci/travis/37-latest-conda-forge.yaml +++ b/ci/travis/37-latest-conda-forge.yaml @@ -21,3 +21,4 @@ dependencies: - geopy - SQLalchemy - psycopg2 + - libspatialite diff --git a/ci/travis/37-latest-defaults.yaml b/ci/travis/37-latest-defaults.yaml index 0bdb398..0cf0ba8 100644 --- a/ci/travis/37-latest-defaults.yaml +++ b/ci/travis/37-latest-defaults.yaml @@ -21,6 +21,7 @@ dependencies: #- geopy - SQLalchemy - psycopg2 + - libspatialite - pip: - codecov - geopy diff --git a/geopandas/geodataframe.py b/geopandas/geodataframe.py index a6363c9..5ab9325 100644 --- a/geopandas/geodataframe.py +++ b/geopandas/geodataframe.py @@ -145,7 +145,7 @@ class GeoDataFrame(GeoPandasBase, DataFrame): level.crs = crs # Check that we are using a listlike of geometries - if not all(isinstance(item, BaseGeometry) or not item for item in level): + if not all(isinstance(item, BaseGeometry) or pd.isnull(item) for item in level): raise TypeError("Input geometry column must contain valid geometry objects.") frame[geo_column_name] = level frame._geometry_column_name = geo_column_name @@ -239,10 +239,9 @@ class GeoDataFrame(GeoPandasBase, DataFrame): @classmethod def from_postgis(cls, sql, con, geom_col='geom', crs=None, - hex_encoded=True, index_col=None, coerce_float=True, - params=None): + index_col=None, coerce_float=True, params=None): """Alternate constructor to create a ``GeoDataFrame`` from a sql query - containing a geometry column. + containing a geometry column in WKB representation. Parameters ---------- @@ -252,9 +251,6 @@ class GeoDataFrame(GeoPandasBase, DataFrame): column name to convert to shapely geometries crs : optional Coordinate reference system to use for the returned GeoDataFrame - hex_encoded : bool, optional - Whether the geometry is in a hex-encoded string. Default is True, - standard for postGIS. index_col : string or list of strings, optional, default: None Column(s) to set as index(MultiIndex) coerce_float : boolean, default True @@ -265,12 +261,15 @@ class GeoDataFrame(GeoPandasBase, DataFrame): Examples -------- - >>> sql = "SELECT geom, highway FROM roads;" + PostGIS + >>> sql = "SELECT geom, highway FROM roads" + SpatiaLite + >>> sql = "SELECT ST_Binary(geom) AS geom, highway FROM roads" >>> df = geopandas.GeoDataFrame.from_postgis(sql, con) """ df = geopandas.io.sql.read_postgis( - sql, con, geom_col=geom_col, crs=crs, hex_encoded=hex_encoded, + sql, con, geom_col=geom_col, crs=crs, index_col=index_col, coerce_float=coerce_float, params=params) return df diff --git a/geopandas/io/sql.py b/geopandas/io/sql.py index 8b3e80b..26457f6 100644 --- a/geopandas/io/sql.py +++ b/geopandas/io/sql.py @@ -1,14 +1,15 @@ +import sys import pandas as pd import shapely.wkb from geopandas import GeoDataFrame -def read_postgis(sql, con, geom_col='geom', crs=None, hex_encoded=True, - index_col=None, coerce_float=True, params=None): +def read_postgis(sql, con, geom_col='geom', crs=None, index_col=None, + coerce_float=True, params=None): """ Returns a GeoDataFrame corresponding to the result of the query - string, which must contain a geometry column. + string, which must contain a geometry column in WKB representation. Parameters ---------- @@ -23,9 +24,6 @@ def read_postgis(sql, con, geom_col='geom', crs=None, hex_encoded=True, CRS to use for the returned GeoDataFrame; if not set, tries to determine CRS from the SRID associated with the first geometry in the database, and assigns that to all geometries. - hex_encoded : bool, optional - Whether the geometry is in a hex-encoded string. Default is True, - standard for postGIS. Use hex_encoded=False for sqlite databases. See the documentation for pandas.read_sql for further explanation of the following parameters: @@ -37,7 +35,10 @@ def read_postgis(sql, con, geom_col='geom', crs=None, hex_encoded=True, Example ------- - >>> sql = "SELECT geom, kind FROM polygons;" + PostGIS + >>> sql = "SELECT geom, kind FROM polygons" + SpatiaLite + >>> sql = "SELECT ST_AsBinary(geom) AS geom, kind FROM polygons" >>> df = geopandas.read_postgis(sql, con) """ @@ -47,17 +48,33 @@ def read_postgis(sql, con, geom_col='geom', crs=None, hex_encoded=True, if geom_col not in df: raise ValueError("Query missing geometry column '{}'".format(geom_col)) - def load_geom(x): - if isinstance(x, bytes): - return shapely.wkb.loads(x, hex=hex_encoded) - else: - return shapely.wkb.loads(str(x), hex=hex_encoded) - geoms = df[geom_col].apply(load_geom) - df[geom_col] = geoms + geoms = df[geom_col].dropna() - if crs is None: - if len(geoms) > 0: - srid = shapely.geos.lgeos.GEOSGetSRID(geoms[0]._geom) + if not geoms.empty: + load_geom_bytes = shapely.wkb.loads + """Load from Python 3 binary.""" + + def load_geom_buffer(x): + """Load from Python 2 binary.""" + return shapely.wkb.loads(str(x)) + + def load_geom_text(x): + """Load from binary encoded as text.""" + return shapely.wkb.loads(str(x), hex=True) + + if sys.version_info.major < 3: + if isinstance(geoms.iat[0], buffer): + load_geom = load_geom_buffer + else: + load_geom = load_geom_text + elif isinstance(geoms.iat[0], bytes): + load_geom = load_geom_bytes + else: + load_geom = load_geom_text + + df[geom_col] = geoms = geoms.apply(load_geom) + if crs is None: + srid = shapely.geos.lgeos.GEOSGetSRID(geoms.iat[0]._geom) # if no defined SRID in geodatabase, returns SRID of 0 if srid != 0: crs = {"init": "epsg:{}".format(srid)} diff --git a/geopandas/io/tests/test_io.py b/geopandas/io/tests/test_io.py index 3a458da..a85f3f1 100644 --- a/geopandas/io/tests/test_io.py +++ b/geopandas/io/tests/test_io.py @@ -2,6 +2,7 @@ from __future__ import absolute_import from collections import OrderedDict +import sys import fiona import pytest from shapely.geometry import box @@ -9,8 +10,7 @@ from shapely.geometry import box import geopandas from geopandas import read_postgis, read_file from geopandas.io.file import fiona_env -from geopandas.tests.util import connect, create_postgis, validate_boro_df - +from geopandas.tests.util import connect, connect_spatialite, create_spatialite, create_postgis, validate_boro_df @pytest.fixture def nybb_df(): @@ -110,6 +110,39 @@ class TestIO: validate_boro_df(df) assert(df.crs == orig_crs) + def test_read_postgis_null_geom(self): + """Tests that geometry with NULL is accepted.""" + try: + con = connect_spatialite() + except Exception: + raise pytest.skip() + else: + geom_col = self.df.geometry.name + self.df.geometry.iat[0] = None + create_spatialite(con, self.df) + sql = 'SELECT ogc_fid, borocode, boroname, shape_leng, shape_area, AsEWKB("{0}") AS "{0}" FROM nybb'.format(geom_col) + df = read_postgis(sql, con, geom_col=geom_col) + validate_boro_df(df) + finally: + if 'con' in locals(): + con.close() + + def test_read_postgis_binary(self): + """Tests that geometry read as binary is accepted.""" + try: + con = connect_spatialite() + except Exception: + raise pytest.skip() + else: + geom_col = self.df.geometry.name + create_spatialite(con, self.df) + sql = 'SELECT ogc_fid, borocode, boroname, shape_leng, shape_area, ST_AsBinary("{0}") AS "{0}" FROM nybb'.format(geom_col) + df = read_postgis(sql, con, geom_col=geom_col) + validate_boro_df(df) + finally: + if 'con' in locals(): + con.close() + def test_read_file(self): df = self.df.rename(columns=lambda x: x.lower()) validate_boro_df(df) diff --git a/geopandas/tests/util.py b/geopandas/tests/util.py index 741dece..2bb13d0 100644 --- a/geopandas/tests/util.py +++ b/geopandas/tests/util.py @@ -1,10 +1,11 @@ import os.path +import sys import sqlite3 -import shapely.wkb from geopandas import GeoDataFrame from geopandas.testing import ( geom_equals, geom_almost_equals, assert_geoseries_equal) # flake8: noqa +from pandas import Series HERE = os.path.abspath(os.path.dirname(__file__)) PACKAGE_DIR = os.path.dirname(os.path.dirname(HERE)) @@ -37,7 +38,7 @@ def validate_boro_df(df, case_sensitive=False): else: for col in columns: assert col.lower() in (dfcol.lower() for dfcol in df.columns) - assert all(df.geometry.type == 'MultiPolygon') + assert Series(df.geometry.type).dropna().eq('MultiPolygon').all() def connect(dbname): @@ -48,30 +49,70 @@ def connect(dbname): return con +def get_srid(df): + """Return srid from `df.crs`.""" + crs = df.crs + return (int(crs['init'][5:]) if 'init' in crs + and crs['init'].startswith('epsg:') + else 0) -def create_sqlite(df, filename, geom_col="geom"): +def connect_spatialite(): """ - Create a sqlite database with the nybb table. This was the result of - using ogr2ogr to create a sqlite database from a shapefile, and then - the .dump command within sqlite to produce the commands to reproduce the - database, so may not representative of standard sqlite databases. + Return a memory-based SQLite3 connection with SpatiaLite enabled & initialized. + + `The sqlite3 module must be built with loadable extension support `_ and `SpatiaLite `_ must be available on the system as a SQLite module. + Packages available on Anaconda meet requirements. + + Exceptions + ---------- + ``AttributeError`` on missing support for loadable SQLite extensions + ``sqlite3.OperationalError`` on missing SpatiaLite + """ + try: + with sqlite3.connect(':memory:') as con: + con.enable_load_extension(True) + con.load_extension('mod_spatialite') + con.execute('SELECT InitSpatialMetaData(TRUE)') + except Exception: + con.close() + raise + return con + +def create_spatialite(con, df): + """ + Return a SpatiaLite connection containing the nybb table. + + Parameters + ---------- + `con`: ``sqlite3.Connection`` + `df`: ``GeoDataFrame`` """ - con = sqlite3.connect(filename) - cur = con.cursor() - cur.execute("CREATE TABLE IF NOT EXISTS 'nybb' " - "( ogc_fid INTEGER PRIMARY KEY AUTOINCREMENT, " - "'{}' BLOB, 'borocode' INTEGER, ".format(geom_col) + - "'boroname' VARCHAR(32), 'shape_leng' FLOAT, " - "'shape_area' FLOAT);") - sql_row = "INSERT INTO nybb VALUES({},X'{}',{},'{}',{},{});" - for i, row in df.iterrows(): - cur.execute(sql_row.format(i, shapely.wkb.dumps(row['geometry'], - hex=True), - row['BoroCode'], row['BoroName'], - row['Shape_Leng'], row['Shape_Area'])) - con.commit() - con.close() + with con: + geom_col = df.geometry.name + srid = get_srid(df) + con.execute('CREATE TABLE IF NOT EXISTS nybb ' + '( ogc_fid INTEGER PRIMARY KEY' + ', borocode INTEGER' + ', boroname TEXT' + ', shape_leng REAL' + ', shape_area REAL' + ')') + con.execute('SELECT AddGeometryColumn(?, ?, ?, ?)', + ('nybb', geom_col, srid, df.geom_type.dropna().iat[0].upper())) + con.execute('SELECT CreateSpatialIndex(?, ?)', ('nybb', geom_col)) + sql_row = "INSERT INTO nybb VALUES(?, ?, ?, ?, ?, GeomFromText(?, ?))" + con.executemany(sql_row, + ((None, + row.BoroCode, + row.BoroName, + row.Shape_Leng, + row.Shape_Area, + row.geometry.wkt if row.geometry + else None, + srid + ) for row in df.itertuples(index=False))) + return con def create_postgis(df, srid=None, geom_col="geom"):