Skip to content

Commit

Permalink
Remove numpy dependency and simplify opener registration
Browse files Browse the repository at this point in the history
Turns out that the shapefile driver, at least, opens files in r+
mode as well as w mode. Removing the mode from the registry key
makes the whole implementation more simple, too.
  • Loading branch information
sgillies committed Mar 4, 2024
1 parent aa3c124 commit a8cebf8
Show file tree
Hide file tree
Showing 8 changed files with 58 additions and 32 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ jobs:
run: |
conda config --prepend channels conda-forge
conda config --set channel_priority strict
conda create -n test python=${{ matrix.python-version }} libgdal geos=3.10.3 cython=3 "numpy>=1.25,<2"
conda create -n test python=${{ matrix.python-version }} libgdal geos=3.10.3 cython=3
source activate test
python -m pip install -e . || python -m pip install -e .
python -m pip install -r requirements-dev.txt
Expand All @@ -120,7 +120,7 @@ jobs:
run: |
conda config --prepend channels conda-forge
conda config --set channel_priority strict
conda create -n test python=${{ matrix.python-version }} libgdal geos=3.10.3 cython=3 "numpy>=1.25,<2"
conda create -n test python=${{ matrix.python-version }} libgdal geos=3.10.3 cython=3
source activate test
GDAL_VERSION="3.5" python setup.py build_ext -I"C:\\Miniconda\\envs\\test\\Library\\include" -lgdal_i -L"C:\\Miniconda\\envs\\test\\Library\\lib" install
python -m pip install -r requirements-dev.txt
Expand Down
1 change: 0 additions & 1 deletion environment.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ dependencies:
- python=3.9.*
- libgdal=3.4.*
- cython=3
- numpy>=1.25,<2
- sphinx-click
- sphinx-rtd-theme
- pip:
Expand Down
4 changes: 2 additions & 2 deletions fiona/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -300,8 +300,8 @@ def func(*args, **kwds):

try:
if opener:
log.debug("Registering opener: raw_dataset_path=%r, mode=%r, opener=%r", raw_dataset_path, mode, opener)
vsi_path_ctx = _opener_registration(raw_dataset_path, mode[0], opener)
log.debug("Registering opener: raw_dataset_path=%r, opener=%r", raw_dataset_path, mode, opener)
vsi_path_ctx = _opener_registration(raw_dataset_path, opener)
registered_vsi_path = stack.enter_context(vsi_path_ctx)
log.debug("Registered vsi path: registered_vsi_path%r", registered_vsi_path)
path = _UnparsedPath(registered_vsi_path)
Expand Down
45 changes: 27 additions & 18 deletions fiona/_vsiopener.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ from urllib.parse import urlparse
from uuid import uuid4

from libc.string cimport memcpy
cimport numpy as np

from fiona.errors import OpenerRegistrationError

Expand Down Expand Up @@ -52,6 +51,7 @@ cdef int install_pyopener_plugin(VSIFilesystemPluginCallbacksStruct *callbacks_s
callbacks_struct.seek = <VSIFilesystemPluginSeekCallback>pyopener_seek
callbacks_struct.read = <VSIFilesystemPluginReadCallback>pyopener_read
callbacks_struct.write = <VSIFilesystemPluginWriteCallback>pyopener_write
callbacks_struct.flush = <VSIFilesystemPluginFlushCallback>pyopener_flush
callbacks_struct.close = <VSIFilesystemPluginCloseCallback>pyopener_close
callbacks_struct.read_dir = <VSIFilesystemPluginReadDirCallback>pyopener_read_dir
callbacks_struct.stat = <VSIFilesystemPluginStatCallback>pyopener_stat
Expand Down Expand Up @@ -83,11 +83,7 @@ cdef int pyopener_stat(
urlpath = pszFilename.decode("utf-8")
parsed_uri = urlparse(urlpath)
parent = Path(parsed_uri.path).parent

# Note that "r" mode is used here under the assumption that GDAL
# doesn't read_dir when writing data. Could be wrong!
mode = "r"
key = ((parsed_uri.scheme, parsed_uri.netloc, parent.as_posix()), mode)
key = (parsed_uri.scheme, parsed_uri.netloc, parent.as_posix())

registry = _OPENER_REGISTRY.get()
log.debug("Looking up opener in pyopener_stat: registry=%r, key=%r", registry, key)
Expand Down Expand Up @@ -130,11 +126,7 @@ cdef char ** pyopener_read_dir(
"""Provides a directory listing to GDAL from a Python filesystem."""
urlpath = pszDirname.decode("utf-8")
parsed_uri = urlparse(urlpath)

# Note that "r" mode is used here under the assumption that GDAL
# doesn't read_dir when writing data. Could be wrong!
mode = "r"
key = ((parsed_uri.scheme, parsed_uri.netloc, parsed_uri.path), mode[0])
key = (parsed_uri.scheme, parsed_uri.netloc, parsed_uri.path)

registry = _OPENER_REGISTRY.get()
log.debug("Looking up opener in pyopener_read_dir: registry=%r, key=%r", registry, key)
Expand Down Expand Up @@ -182,7 +174,7 @@ cdef void* pyopener_open(
parsed_uri = urlparse(urlpath)
path_to_check = Path(parsed_uri.path)
parent = path_to_check.parent
key = ((parsed_uri.scheme, parsed_uri.netloc, parent.as_posix()), mode[0])
key = (parsed_uri.scheme, parsed_uri.netloc, parent.as_posix())

registry = _OPENER_REGISTRY.get()
log.debug("Looking up opener in pyopener_open: registry=%r, key=%r", registry, key)
Expand Down Expand Up @@ -261,11 +253,27 @@ cdef size_t pyopener_read(void *pFile, void *pBuffer, size_t nSize, size_t nCoun


cdef size_t pyopener_write(void *pFile, void *pBuffer, size_t nSize, size_t nCount) with gil:
if pBuffer == NULL:
return -1
cdef object file_obj = <object>pFile
buffer_len = nSize * nCount
cdef np.uint8_t [:] buff_view = <np.uint8_t[:buffer_len]>pBuffer
log.debug("Writing data: buff_view=%r", buff_view)
return <size_t>file_obj.write(buff_view)
cdef unsigned char [:] buff_view = <unsigned char[:buffer_len]>pBuffer
log.debug("Writing data: file_obj=%r, buff_view=%r, buffer_len=%r", file_obj, buff_view, buffer_len)
try:
num = file_obj.write(buff_view)
except TypeError:
num = file_obj.write(str(buff_view))
return 1 # <size_t>num

This comment has been minimized.

Copy link
@sgillies

sgillies Mar 4, 2024

Author Member

@rouault can you help me understand what the write callback should return? I assumed it should return the number of bytes or objects written, but I found that a shapefile can't be successfully written unless only 1 is returned.

This comment has been minimized.

Copy link
@rouault

rouault Mar 4, 2024

Contributor

Same as fwrite(). So num // nSize (which should be the same as nCount when writing works fine)

This comment has been minimized.

Copy link
@sgillies

sgillies Mar 4, 2024

Author Member

Thank you!



cdef int pyopener_flush(void *pFile) with gil:
cdef object file_obj = <object>pFile
log.debug("Flushing: file_obj=%r", file_obj)
try:
file_obj.flush()
return 0
except AttributeError:
return 1

This comment has been minimized.

Copy link
@sgillies

sgillies Mar 4, 2024

Author Member

Shapefile requires the flush callback. It's a tough format to support 😅



cdef int pyopener_close(void *pFile) with gil:
Expand All @@ -279,18 +287,19 @@ cdef int pyopener_close(void *pFile) with gil:


@contextlib.contextmanager
def _opener_registration(urlpath, mode, obj):
def _opener_registration(urlpath, obj):
parsed_uri = urlparse(urlpath)
path_to_check = Path(parsed_uri.path)
parent = path_to_check.parent
key = ((parsed_uri.scheme, parsed_uri.netloc, parent.as_posix()), mode[0])
key = (parsed_uri.scheme, parsed_uri.netloc, parent.as_posix())

# Might raise.
opener = _create_opener(obj)

registry = _OPENER_REGISTRY.get()
if key in registry:
if registry[key] != opener:
raise OpenerRegistrationError(f"Opener already registered for urlpath and mode.")
raise OpenerRegistrationError(f"Opener already registered for urlpath.")
else:
try:
yield f"{PREFIX}{urlpath}"
Expand Down
3 changes: 1 addition & 2 deletions pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
[build-system]
requires = ["setuptools>=67.8", "cython~=3.0.2", "numpy>=1.25,<2"]
requires = ["setuptools>=67.8", "cython~=3.0.2"]
build-backend = "setuptools.build_meta"

[project]
Expand Down Expand Up @@ -35,7 +35,6 @@ dependencies = [
"click-plugins>=1.0",
"cligj>=0.5",
'importlib-metadata;python_version<"3.10"',
"numpy<2",
]

[project.optional-dependencies]
Expand Down
1 change: 0 additions & 1 deletion requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,4 @@ click-plugins
cligj>=0.5.0
importlib-metadata;python_version<"3.10"
munch>=2.3.2
numpy>=1.25,<2
certifi
6 changes: 0 additions & 6 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,6 @@ def copy_data_tree(datadir, destdir):
gdal_major_version = 0
gdal_minor_version = 0

try:
import numpy as np
include_dirs.append(np.get_include())
except ImportError:
raise SystemExit("ERROR: Numpy and its headers are required to run setup().")

if 'clean' not in sys.argv:
try:
gdal_config = os.environ.get('GDAL_CONFIG', 'gdal-config')
Expand Down
26 changes: 26 additions & 0 deletions tests/test_pyopener.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import pytest

import fiona
from fiona.model import Feature


def test_opener_io_open(path_grenada_geojson):
Expand Down Expand Up @@ -52,3 +53,28 @@ def test_opener_tiledb_file():
assert len(colxn) == 67
assert colxn.schema["geometry"] == "Polygon"
assert "AGBUR" in colxn.schema["properties"]


def test_opener_fsspec_fs_write(tmp_path):
"""Write a feature via an fsspec fs opener."""
schema = {"geometry": "Point", "properties": {"zero": "int"}}
feature = Feature.from_dict(
**{
"geometry": {"type": "Point", "coordinates": (0, 0)},
"properties": {"zero": "0"},
}
)
fs = fsspec.filesystem("file")
outputfile = tmp_path.joinpath("test.shp")

with fiona.open(
str(outputfile),
"w",
driver="ESRI Shapefile",
schema=schema,
crs="OGC:CRS84",
opener=fs,
) as collection:
collection.write(feature)
assert len(collection) == 1
assert collection.crs == "OGC:CRS84"

0 comments on commit a8cebf8

Please sign in to comment.