-
-
Notifications
You must be signed in to change notification settings - Fork 700
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
Attempt to fix doctests by using airports layer in epsg4326 instead of epsg2964. #9256
base: master
Are you sure you want to change the base?
Attempt to fix doctests by using airports layer in epsg4326 instead of epsg2964. #9256
Conversation
…f epsg2964. Use geopackage airports layer (epsg4326) to avoid ambiguous reprojection warning of previously used Alaska scoped airports layer (epsg2964) when adding it programmatically to a vanilla qgis project during doctest. We do not simply reproject the layer, because the alaska airports crs is mentioned mutliple times in the codebase and streamlining the whole cookbook is out of scope for this PR.
@selmaVH1 could you please trigger the doctest workflow to see if it passes? If not, the whole PR is pointless. If it passes a few times in a row, a priority review/merge could benefit all other PRs. Cheers! |
Cool thanks! The workflow failed, but not due to failing tests (which is good) but due to a segfault after running the tests (which is bad). Link. I am not sure what this is about and how it is connected to my changes. If anybody has an idea, ping me. From failed workflow run logs:
|
…nto fix_doctest_by_using_epsg4326_dataset_instead_of_epsg2964
Hi @folinimarc, thank you for your work on this! I know that airport.shp has been part of the sample data for a long time, and while I'm not entirely sure, I don't think it has previously caused issues or doctest failures. But I do support preferring geopackage over shapefile. @timlinux, if you have time to review this PR, your input would be very helpful. 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 @folinimarc for addressing this annoying issue.
We could have used an airports_4326.shp layer but I'm fine with using gpkg. However I miss an example with shp and think it could be possible to add one, as untested code.
@@ -49,28 +49,7 @@ Vector Layers | |||
============= | |||
|
|||
To create and add a vector layer instance to the project, specify the layer's data source | |||
identifier, name for the layer and provider's name: |
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.
It is unclear to me why layer name and provider are removed here. and "layer name" is mentioned two sentences after.
path to the file: | ||
* The ogr provider from the GDAL library supports a `wide variety of formats <https://gdal.org/en/latest/drivers/vector/index.html>`_, | ||
also called drivers in GDAL speak. | ||
Examples are Geopackage, Flatgeobuf, Geojson and also The-One-We-Shall-Not-Name. |
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.
Examples are Geopackage, Flatgeobuf, Geojson and also The-One-We-Shall-Not-Name. | |
Examples are Geopackage, Flatgeobuf, Geojson and also ESRI Shapefile. |
Not everyone knows what it is about so let's name it, please.
@@ -1230,7 +1237,7 @@ arrangement) | |||
|
|||
from qgis.PyQt import QtGui | |||
|
|||
myVectorLayer = QgsVectorLayer("testdata/airports.shp", "airports", "ogr") | |||
myVectorLayer = QgsVectorLayer("testdata/data/data.gpkg|layername=airports", "Airports layer", "ogr") | |||
myTargetField = 'ELEV' |
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.
This field doesn't exist in the gpkg, does it?
|
||
* for Shapefile: |
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.
Imo It is a pity to no longer provide an example with flat files. We can keep the gpkg for tests and it should be possible to add a .. code::
sample for shp without putting it in the testing workflow. And it could appear at the beginning, new line 75.
Not ideal but would address the simplest and most occurring situation.
@@ -96,7 +74,7 @@ method of the :class:`QgisInterface <qgis.gui.QgisInterface>`: | |||
|
|||
.. testcode:: loadlayer | |||
|
|||
vlayer = iface.addVectorLayer(path_to_airports_layer, "Airports layer", "ogr") | |||
vlayer = iface.addVectorLayer(gpkg_airports_layer, "Airports layer", "ogr") |
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.
In reference to my comment about missing shp, we could keep shp code here...
@@ -79,12 +58,11 @@ For a geopackage vector layer: | |||
|
|||
.. testcode:: loadlayer |
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.
Do we really need to keep this example here? Before we ever mention the addVectorLayer method? and knowing that it is a more verbose version of new line 101?
I know you are simply following the existing docs
Fixes #9255
This is an attempt to fix the flaky doctests behavior, only actual workflow runs on the Github agent will provide certainty.
Primary changes
Secondary changes