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.

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

Add parameter to skip layername in gdal:convertformat #45955

Merged
merged 10 commits into from
Dec 9, 2021

Conversation

benoitblanc
Copy link
Contributor

Description

In some use cases, user needs to export a whole source to an other format, using gdal:convertformat (ogr2ogr.py) but the processing only converts one layer at the moment.

For example, I want to export a whole schema from a database to a file :

INPUT: "PG:service='test' sslmode=disable" public.batiment
OUTPUT: /tmp/OUTPUT.gpkg

GDAL command is : ogr2ogr -f "GPKG" /tmp/OUTPUT.gpkg "PG:service='test' sslmode=disable" public.batiment. Only batiment table has been converted.

If I add a IGNORE_LAYERNAME parameter to True :

INPUT: "PG:service='test' sslmode=disable" public.batiment
IGNORE_LAYERNAME: True
OUTPUT: /tmp/OUTPUT.gpkg

GDAL command is : ogr2ogr -f "GPKG" /tmp/OUTPUT.gpkg "PG:service='test' sslmode=disable". Whole schema is converted.

IGNORE_LAYERNAME is a boolean parameter which default value is False so the default behavior of importing only one layer is kept.

This pull request superseds #39580

@github-actions github-actions bot added this to the 3.24.0 milestone Nov 9, 2021
@benoitblanc benoitblanc changed the title Add parameter to skip layername in gdal:convertformat [backport release-3_16] Add parameter to skip layername in gdal:convertformat Nov 9, 2021
@nyalldawson nyalldawson changed the title [backport release-3_16] Add parameter to skip layername in gdal:convertformat Add parameter to skip layername in gdal:convertformat Nov 9, 2021
@nyalldawson
Copy link
Collaborator

Sounds like a useful feature!

I'm wondering, instead of "ignore layer name" if it would be more understandable to go with "convert all layers" or "convert entire dataset"?

In either case it needs some documentation in the short help string and also a unit test of the new parameter.

@alexbruy alexbruy added Processing Relating to QGIS Processing framework or individual Processing algorithms Requires Tests! Waiting on the submitter to add unit tests before eligible for merging Needs Documentation When merging a labeled PR, an issue will be created in the Doc repo. labels Nov 10, 2021
@github-actions
Copy link

@benoitblanc
This pull request has been tagged as requiring documentation.

A documentation ticket will be opened at https://github.com/qgis/QGIS-Documentation when this PR is merged.

Please update the description (not the comments) with helpful description and screenshot to help the work from documentors.
Also, any commit having [needs-doc] or [Needs Documentation] in will see its message pushed to the issue, so please be as verbose as you can.

Thank you!

@alexbruy
Copy link
Contributor

Will this work with formats other than GPKG and how loading of the output layer will be handled in a such case?

@gioman gioman added the Feedback Waiting on the submitter for answers label Nov 11, 2021
@benoitblanc
Copy link
Contributor Author

benoitblanc commented Nov 16, 2021

Yes this works with other formats than GPKG. I have added tests to convert one layer or all layers from a GPKG file to GML files. When all layers are converted, you can open the file in QGIS and chose layers you want to add in your project.

I think there might be other formats that supports FeatureCollection which can be used here such as KML or GeoJSON.

Edit: It doesn't work with GeoJSON because driver can not create layer.

ogr2ogr -f "GeoJSON" test.geojson points_lines.gpkg --debug ON
GPKG: GeoPackage v1.2.0
GDAL: GDALOpen(points_lines.gpkg, this=0x55cf645a1bf0) succeeds as GPKG.
GDAL: GDALDriver::Create(GeoJSON,test.geojson,0,0,0,Unknown,(nil))
GPKG: ResetStatement(SELECT m."fid", m."geom", m."id", m."id2" FROM "points" m)
GPKG: finalize 0x55cf645ccb48
GDALVectorTranslate: 9 features written in layer 'points'
ERROR 1: Layer 'lines' does not already exist in the output dataset, and cannot be created by the output driver.
ERROR 1: Terminating translation prematurely after failed
translation of layer lines (use -skipfailures to skip errors)
GDAL: GDALClose(test.geojson, this=0x55cf645d0c70)
GDAL: GDALClose(points_lines.gpkg, this=0x55cf645a1bf0)

@alexbruy
Copy link
Contributor

alexbruy commented Nov 23, 2021

Edit: It doesn't work with GeoJSON because driver can not create layer.

Then this should be mentioned in the docs (probably with the list of the supported formats) and maybe it is worth at least evaluate a possibility to check at runtime whether output format can be used and if this is possible then raise error.

@benoitblanc benoitblanc force-pushed the ogr2ogr_ignore_layername branch from 553d2c4 to d1ec369 Compare November 30, 2021 13:52
@nyalldawson
Copy link
Collaborator

I'm happy with the current revision!

@alexbruy @DelazJ any objections to merge? any further changes desired?

@alexbruy
Copy link
Contributor

alexbruy commented Dec 2, 2021

@nyalldawson looks ok for me. While I think it would be good to have run-time check whether output format allows saving multiple layers or no, we can merge it now and add that check later if needed.

@DelazJ
Copy link
Contributor

DelazJ commented Dec 2, 2021

Looks OK to me. Thanks.

@nyalldawson nyalldawson enabled auto-merge (rebase) December 8, 2021 23:53
@github-actions
Copy link

github-actions bot commented Dec 9, 2021

@benoitblanc
A documentation ticket has been opened at qgis/QGIS-Documentation#7197
It is your responsibility to visit this ticket and add as much detail as possible for the documentation team to correctly document this change.
Thank you!

@agiudiceandrea
Copy link
Member

Fixes #43701

@agiudiceandrea
Copy link
Member

agiudiceandrea commented Feb 21, 2022

@benoitblanc @alexbruy

how loading of the output layer will be handled in a such case?

In fact, there is a issue here: when a multi layer GeoPackage (or other multilayer format, like GML, KML, Spatialite, SQLite) file is created by the algorithm and the "Open output file after running algorithm" is checked (as default), then only 1 layer is automatically added to the map instead of all the layer contained in the output multi layer GeoPackage file.

This could give the user the false impression that the conversion process has not happened correctly for all the layers contained in the source multi layer GeoPackage file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feedback Waiting on the submitter for answers Needs Documentation When merging a labeled PR, an issue will be created in the Doc repo. Processing Relating to QGIS Processing framework or individual Processing algorithms Requires Tests! Waiting on the submitter to add unit tests before eligible for merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants