-
-
Notifications
You must be signed in to change notification settings - Fork 25
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
REF: factor out dataset and layer creation for ogr_write #351
REF: factor out dataset and layer creation for ogr_write #351
Conversation
There is one failing build with GDAL main, but that's being addressed in #353 |
Thanks! |
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.
Thanks for working on this!
Just a couple of comments...
pyogrio/_io.pyx
Outdated
object layer_kwargs, bint promote_to_multi=False, bint nan_as_null=True, | ||
bint append=False, dataset_metadata=None, layer_metadata=None, | ||
gdal_tz_offsets=None | ||
cdef create_dataset_layer( |
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.
Can you please add a minimal docstring to this to indicate it returns a boolean to indicate that a new layer is created in the process (vs append to existing), that it sets ogr_dataset
and ogr_layer
pointers with valid objects on success and raise exceptions otherwise, and that it is then the responsibility of the caller to release those?
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.
Sure!
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.
Thanks!
This just moves the setting up of the ogr dataset and layer objects for writing into a separate helper function, which should help with #346