<div dir="ltr"><div>Hi Even,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Nov 11, 2019 at 7:21 AM Even Rouault <<a href="mailto:even.rouault@spatialys.com" target="_blank">even.rouault@spatialys.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On mercredi 6 novembre 2019 00:02:52 CET Mateusz Loskot wrote:<br>
> On Tue, 5 Nov 2019 at 12:06, Even Rouault <<a href="mailto:even.rouault@spatialys.com" target="_blank">even.rouault@spatialys.com</a>> wrote:<br>
> > This is a topic we already discussed a couple years ago. Please find a RFC<br>
> > about it in:<br>
> > <br>
> > <a href="https://github.com/OSGeo/gdal/pull/1984/files" rel="noreferrer" target="_blank">https://github.com/OSGeo/gdal/pull/1984/files</a><br>
> <br>
> I dropped some comments there, not a review though.<br>
<br>
I've updated both the RFC and the implementation (number of changes since the<br>
preliminary one) to what I believe should be close to their final states.<br>
<br>
In addition to the rather dummy example of a Python driver that is in the RFC,<br>
there are 2 more interesting examples:<br>
<br>
* a PASSTHROUGH driver that forwards calls to the GDAL SWIG Python API:<br>
<a href="https://github.com/rouault/gdal/blob/pythondrivers/gdal/examples/pydrivers/ogr_PASSTHROUGH.py" rel="noreferrer" target="_blank">https://github.com/rouault/gdal/blob/pythondrivers/gdal/examples/pydrivers/ogr_PASSTHROUGH.py</a><br>
<br>
* a driver implemented a simple parsing of CityJSON (<a href="https://www.cityjson.org/" rel="noreferrer" target="_blank">https://www.cityjson.org/</a>)<br>
<a href="https://github.com/rouault/gdal/blob/pythondrivers/gdal/examples/pydrivers/ogr_CityJSON.py" rel="noreferrer" target="_blank">https://github.com/rouault/gdal/blob/pythondrivers/gdal/examples/pydrivers/ogr_CityJSON.py</a><br>
<br>
The tutorialshould also help:<br>
<a href="https://github.com/rouault/gdal/blob/pythondrivers/gdal/doc/source/tutorials/vector_python_driver.rst" rel="noreferrer" target="_blank">https://github.com/rouault/gdal/blob/pythondrivers/gdal/doc/source/tutorials/vector_python_driver.rst</a><br>
<br>
I'll submit the RFC to vote soon if there are no other remarks.<br>
<br>
Even<br></blockquote><div><br></div><div>On the one hand, If I were a MapServer site administrator and it was my job to render features from some special non-standard or proprietary format or API as a layer on my maps and all it took was one Python file copied into a directory on my server, I think I might like this feature. I feel like there are already too many format drivers in GDAL/OGR and anything that keeps a driver that only a handful of people will ever use out of the core of the library is a good thing. On the other hand, I feel like these plugins are a complicated solution to a problem that is pretty well solved by running a format-specific python (or other language) program that writes a stream of GeoJSON texts.</div><div><br></div><div>That said I have some comments that I hope will help refine the RFC.</div><div><br></div><div>About <a href="https://github.com/rouault/gdal/blob/pythondrivers/gdal/doc/source/tutorials/vector_python_driver.rst#driver-location" target="_blank">https://github.com/rouault/gdal/blob/pythondrivers/gdal/doc/source/tutorials/vector_python_driver.rst#driver-location</a>, can you expand on the distribution/installation story for these plugins? Will there be any guidance about plugin dependencies? Will GDAL resolve and fetch them? Should they be vendored into the plugin?</div><div><br></div><div>About <a href="https://github.com/rouault/gdal/blob/pythondrivers/gdal/doc/source/tutorials/vector_python_driver.rst#metadata-section" target="_blank">https://github.com/rouault/gdal/blob/pythondrivers/gdal/doc/source/tutorials/vector_python_driver.rst#metadata-section</a>, since the metadata are not likely to be used in the Python code, might they be defined at the top of the file in a commented line? Cython directives are a good example: <a href="https://cython.readthedocs.io/en/latest/src/userguide/source_files_and_compilation.html#globally" target="_blank">https://cython.readthedocs.io/en/latest/src/userguide/source_files_and_compilation.html#globally</a>.</div><div><br></div><div>About the Driver interface, relying on drivers to register themselves as the handlers for filenames (with "identify") seems to me to open the door to conflicts and tricky bugs. Have you considered an XML (or whatever) file in the plugin directory as a registry? </div><div><br></div><div>About the first bytes of the "file", what cases are you thinking of where a developer wouldn't be able to get them in the code they write? And how many bytes are we talking about?</div><div><br></div><div>About the Layer interface, I feel like instead of offering two flavors: one based on GeoJSON and one based on the GDAL bindings, offer only one. Best practices will emerge faster, I think.</div></div><div><br></div><div><div>From what I see in the passthrough example, it looks like much of the "magic" of these plugins is going to be undone by requirements to prefix the connection strings with the name of the plugin driver. Is `ogrinfo PASSTHROUGH:myfile` any improvement over `python passthrough.py myfile`?</div><div></div></div><div><br></div>-- <br><div dir="ltr"><div dir="ltr">Sean Gillies</div></div></div>