-
Notifications
You must be signed in to change notification settings - Fork 213
Expose driver metadata, refactor dataset creation #909
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.
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
Expose driver metadata, refactor dataset creation #909
Conversation
fiona/ogrext.pyx
Outdated
| if k.upper() in driver_dataset_open_options: | ||
| open_kwargs[k] = v | ||
| else: | ||
| log.debug("Ignore '{}' as dataset open option.".format(k)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would a log.info or log.warn be more appropriate instead of log.debug?
| GDALClose(cogr_ds) | ||
| else: | ||
| cogr_driver = OGRGetDriverByName(driver.encode("utf-8")) | ||
| cogr_driver = exc_wrap_pointer(GDALGetDriverByName(driver.encode("utf-8"))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OGRGetDriverByName is deprecated
|
|
||
| metadata = "" | ||
| cogr_driver = exc_wrap_pointer(GDALGetDriverByName(driver.encode('utf-8'))) | ||
| metadata_c = GDALGetMetadataItem(cogr_driver, strencode(metadata_item), NULL) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, only the default domain is used (NULL). The domains can be queried with GDALGetMetadataDomainList, however for the drivers I tested it with only the default domain (NULL) was reported. I have to admit I don't yet fully understand the domain concept and where it is used.
|
I suppose the driver metadata, especially for ogr drivers, is previously not used that often. Or at least there were quite a few issues with the XML: OSGeo/gdal@f447a31 I'm thus a bit unsure if it is a good idea to use it for filtering. On the other hand, Fiona does not include many "exotic" drivers, thus I suppose the quality of the XML is probably better for them. |
|
Closed in favor of #950 |
Since GDAL 2.0, OGR drivers include more metadata. Currently, there is no way to access this metadata (e.g. to query supported dataset open options, dataset creation options, layer creation options, file extensions etc.) through Fiona. This PR adds a new meta module that allows to query metadata. I decided to create a new module as ogrext seems to grow quite large. If you think that this functionality should belong in Fiona we should think about the best architecture (which might be sharable with rasterio).
File extensions could be used to create in memory datasets, as some drivers have troubles with files without the proper extension (#901). With the current implementation,
fiona.meta.print_driver_options('GeoJSON')outputs the following information:
Knowing the available options allows to filter them in ogrext WritingSession before being passed to gdal_open_vector, gdal_create, respectively GDALDatasetCreateLayer to avoid warnings. (Currently only for write mode, as only there a driver is known). Adding filtering for read and append mode could be added by first querying the driver, such as in _remove() using gdal_open_vector and GDALGetDatasetDriver.
This is related to #504 . However, I'm unsure how much of this issue is already solved with 5e687a0
Adding options filtering to these functions meant to refactor the creation of dataset in the WritingSession. Especially this part needs a careful review. The biggest change involves using _remove() instead of os.unlink() for all drivers instead of just GeoJSON. This allows in the following example that all auxiliary files of a Shapefile are deleted when it is overwritten by a GeoJSON dataset. However, as deleting files is potentially very dangerous, I'm unsure if the default of Fiona should be to overwrite files or to fail. What about adding an overwrite=True/False argument to Collection? The refactoring also fixes #771 and #568
If you think this PR is too big or you would like only a subset of the features, I'm happy to refactor this PR or create a new one.
Documentation of the new features is currently missing, as it is probably best to wait until the implementation is stable before writing it.