Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refactor Geometry classes to subclass the C extension type #983

Merged
merged 27 commits into from
Nov 4, 2020
Merged

Refactor Geometry classes to subclass the C extension type #983

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
2a2cb7c
WIP: use pygeos as base for Geometry classes
jorisvandenbossche Apr 18, 2020
572229e
updates
jorisvandenbossche Sep 10, 2020
7df3d80
fix pickling / unary_union
jorisvandenbossche Sep 16, 2020
04cfb6a
fix CoordinateSequence
jorisvandenbossche Sep 16, 2020
460b06f
fixup or xfail remaining failing tests
jorisvandenbossche Sep 16, 2020
9eaeb16
clean-up some tests
jorisvandenbossche Sep 16, 2020
0631343
add pygeos to travis
jorisvandenbossche Sep 25, 2020
55150c1
wrong GEOS is used to build pygeos
jorisvandenbossche Sep 25, 2020
5fdfe3a
Apply suggestions from code review
jorisvandenbossche Sep 26, 2020
3a81476
try with GEOS_CONFIG env variable
jorisvandenbossche Sep 26, 2020
a28f6da
clean-up - remove unused attributes
jorisvandenbossche Sep 26, 2020
8c847f1
fix Point.z for older GEOS
jorisvandenbossche Sep 26, 2020
afaa0af
debug interpolate failure
jorisvandenbossche Sep 26, 2020
e3cde59
clean-up travis
jorisvandenbossche Sep 29, 2020
984d556
spelling normalize->normalized
jorisvandenbossche Oct 13, 2020
f738a8b
cache CoordinateSequence
jorisvandenbossche Oct 13, 2020
820a5b3
use GEOS is_ccw when available
jorisvandenbossche Oct 13, 2020
fbb982e
remove Point.bounds override
jorisvandenbossche Oct 13, 2020
a913fe9
Install pygeos in appveyor.yml
jorisvandenbossche Oct 13, 2020
8fb974c
set env variables for pygeos
jorisvandenbossche Oct 13, 2020
86ad103
skip failing singularity test
jorisvandenbossche Oct 15, 2020
aca39fd
clean-up print
jorisvandenbossche Oct 15, 2020
646d554
skip test on Appveyor
jorisvandenbossche Nov 4, 2020
3024adc
fix WKT dimensionality test
jorisvandenbossche Nov 4, 2020
41fe614
support big_endian keyword
jorisvandenbossche Nov 4, 2020
ded9d98
skip speedups import test on Appveyor
jorisvandenbossche Nov 4, 2020
e83d073
clean-up
jorisvandenbossche Nov 4, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
45 changes: 4 additions & 41 deletions .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,61 +13,21 @@ matrix:
GEOSVERSION="3.5.2"
SPEEDUPS=1
NUMPY=1
- python: "3.5"
env:
GEOSVERSION="3.5.2"
SPEEDUPS=0
NUMPY=1
- python: "3.5"
env:
GEOSVERSION="3.5.2"
SPEEDUPS=0
NUMPY=0
- python: "3.6"
env:
GEOSVERSION="3.6.4"
SPEEDUPS=1
NUMPY=1
- python: "3.6"
env:
GEOSVERSION="3.6.4"
SPEEDUPS=0
NUMPY=1
- python: "3.6"
env:
GEOSVERSION="3.6.4"
SPEEDUPS=0
NUMPY=0
- python: "3.7"
env:
GEOSVERSION="3.7.3"
SPEEDUPS=1
NUMPY=1
- python: "3.7"
env:
GEOSVERSION="3.7.3"
SPEEDUPS=0
NUMPY=1
- python: "3.7"
env:
GEOSVERSION="3.7.3"
SPEEDUPS=0
NUMPY=0
- python: "3.8"
env:
GEOSVERSION="3.8.1"
SPEEDUPS=1
NUMPY=1
- python: "3.8"
env:
GEOSVERSION="3.8.1"
SPEEDUPS=0
NUMPY=1
- python: "3.8"
env:
GEOSVERSION="3.8.1"
SPEEDUPS=0
NUMPY=0
- python: "3.9-dev"
env:
GEOSVERSION="master"
Expand All @@ -83,6 +43,9 @@ matrix:
before_install:
- ./ci/travis/install_geos.sh
- pip install --upgrade pip
- export PATH=$HOME/geosinstall/geos-$GEOSVERSION/bin/geos-config:$PATH
- export GEOS_CONFIG=$HOME/geosinstall/geos-$GEOSVERSION/bin/geos-config
- pip install git+https://github.com/pygeos/pygeos.git@setup-geos-config -v
# if building with speedups install cython
- if [ "$SPEEDUPS" == "1" ]; then pip install --install-option="--no-cython-compile" cython; fi
# if testing without numpy explicitly remove it
Expand All @@ -99,7 +62,7 @@ script:
- export LD_LIBRARY_PATH=$HOME/geosinstall/geos-$GEOSVERSION/lib
- export DYLD_LIBRARY_PATH=$HOME/geosinstall/geos-$GEOSVERSION/lib
- python -c "from shapely.geos import geos_version; print(geos_version)"
- python -m pytest --cov shapely --cov-report term-missing "${SPEEDUPS_FLAG}"
- python -m pytest --cov shapely --cov-report term-missing "${SPEEDUPS_FLAG}" -s

after_success:
- coveralls || echo "!! intermittent coveralls failure"
Expand Down
126 changes: 30 additions & 96 deletions shapely/coords.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,133 +25,67 @@ class CoordinateSequence(object):

"""

# Attributes
# ----------
# _cseq : c_void_p
# Ctypes pointer to GEOS coordinate sequence
# _ndim : int
# Number of dimensions (2 or 3, generally)
# __p__ : object
# Parent (Shapely) geometry
_cseq = None
_ndim = None
__p__ = None

def __init__(self, parent):
self.__p__ = parent

def _update(self):
self._ndim = self.__p__._ndim
self._cseq = lgeos.GEOSGeom_getCoordSeq(self.__p__._geom)
def __init__(self, coords):
self._coords = coords
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What exactly to do with this CoordinateSequence class is something still to be discussed, and I opened #984 for that

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have been reading up on #984 and I tend to go with this implementation for this PR (making self._coords a ndarray directly)

However I think that sticking a full duplicate of coords to the object by default is a bit too much. Also _coords won't be initialized properly in all construction paths.

Within this approach (I actually like the lazy CoordinateSequency object approach better) I think it is best to use a pattern like this:

@property
def coords(self):
    try:
        coords = getattr(self, "_coords")
    except AttributeError:
        coords = pygeos.get_coordinates(self)
        coords.flags.writeable = False
        self._coords = coords
    return coords

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However I think that sticking a full duplicate of coords to the object by default is a bit too much.

Note that this CoordinateSequence is not constructed by default when creating a geometry object. It will only be created once a user accesses the geom.coords attribute.

Now, with the current code, each time the user does geom.coords, it will create a new CoordinateSequence object containing a new array, so making this "cached" is certainly a good idea (as well as making the array not writable)


def __len__(self):
self._update()
cs_len = c_uint(0)
lgeos.GEOSCoordSeq_getSize(self._cseq, byref(cs_len))
return cs_len.value
return self._coords.shape[0]

def __iter__(self):
self._update()
dx = c_double()
dy = c_double()
dz = c_double()
has_z = self._ndim == 3
for i in range(self.__len__()):
lgeos.GEOSCoordSeq_getX(self._cseq, i, byref(dx))
lgeos.GEOSCoordSeq_getY(self._cseq, i, byref(dy))
if has_z:
lgeos.GEOSCoordSeq_getZ(self._cseq, i, byref(dz))
yield (dx.value, dy.value, dz.value)
else:
yield (dx.value, dy.value)
yield tuple(self._coords[i].tolist())

def __getitem__(self, key):
self._update()
dx = c_double()
dy = c_double()
dz = c_double()
m = self.__len__()
has_z = self._ndim == 3
if isinstance(key, int):
if key + m < 0 or key >= m:
raise IndexError("index out of range")
if key < 0:
i = m + key
else:
i = key
lgeos.GEOSCoordSeq_getX(self._cseq, i, byref(dx))
lgeos.GEOSCoordSeq_getY(self._cseq, i, byref(dy))
if has_z:
lgeos.GEOSCoordSeq_getZ(self._cseq, i, byref(dz))
return (dx.value, dy.value, dz.value)
else:
return (dx.value, dy.value)
return tuple(self._coords[i].tolist())
elif isinstance(key, slice):
res = []
start, stop, stride = key.indices(m)
for i in range(start, stop, stride):
lgeos.GEOSCoordSeq_getX(self._cseq, i, byref(dx))
lgeos.GEOSCoordSeq_getY(self._cseq, i, byref(dy))
if has_z:
lgeos.GEOSCoordSeq_getZ(self._cseq, i, byref(dz))
res.append((dx.value, dy.value, dz.value))
else:
res.append((dx.value, dy.value))
res.append(tuple(self._coords[i].tolist()))
return res
else:
raise TypeError("key must be an index or slice")

@property
def _ctypes(self):
self._update()
has_z = self._ndim == 3
n = self._ndim
m = self.__len__()
array_type = c_double * (m * n)
data = array_type()
temp = c_double()
for i in range(m):
lgeos.GEOSCoordSeq_getX(self._cseq, i, byref(temp))
data[n*i] = temp.value
lgeos.GEOSCoordSeq_getY(self._cseq, i, byref(temp))
data[n*i+1] = temp.value
if has_z:
lgeos.GEOSCoordSeq_getZ(self._cseq, i, byref(temp))
data[n*i+2] = temp.value
return data

def array_interface(self):
"""Provide the Numpy array protocol."""
if sys.byteorder == 'little':
typestr = '<f8'
elif sys.byteorder == 'big':
typestr = '>f8'
else:
raise ValueError(
"Unsupported byteorder: neither little nor big-endian")
ai = {
'version': 3,
'typestr': typestr,
'data': self._ctypes,
}
ai.update({'shape': (len(self), self._ndim)})
return ai

__array_interface__ = property(array_interface)
# def array_interface(self):
# """Provide the Numpy array protocol."""
# if sys.byteorder == 'little':
# typestr = '<f8'
# elif sys.byteorder == 'big':
# typestr = '>f8'
# else:
# raise ValueError(
# "Unsupported byteorder: neither little nor big-endian")
# ai = {
# 'version': 3,
# 'typestr': typestr,
# 'data': self._ctypes,
# }
# ai.update({'shape': (len(self), self._ndim)})
# return ai

# __array_interface__ = property(array_interface)

def __array__(self, dtype=None):
return self._coords

@property
def xy(self):
"""X and Y arrays"""
self._update()
m = self.__len__()
x = array('d')
y = array('d')
temp = c_double()
for i in range(m):
lgeos.GEOSCoordSeq_getX(self._cseq, i, byref(temp))
x.append(temp.value)
lgeos.GEOSCoordSeq_getY(self._cseq, i, byref(temp))
y.append(temp.value)
xy = self._coords[i].tolist()
x.append(xy[0])
y.append(xy[1])
return x, y


Expand Down