Fiona
Fiona copied to clipboard
Add gdal 3.5 to CI, update proj/gdal install scripts to use cmake, update drvsupport
See https://github.com/Toblerity/Fiona/issues/1099#issuecomment-1176004721
This pinned version of munch is incompatible with Python 3.11: https://github.com/Toblerity/Fiona/blob/9d06389755928a3ae4ffa55d4156700f5260ec1c/requirements.txt#L5 not sure which one to pick, but there are several newer versions
Looks like I opened a can of worms.
This PR:
- adds GDAL 3.5 to the CI matrix.
- updates install scripts to use cmake. This is necessary for Proj 9, and will be necessary for GDAL 3.6 (which is on master already).
- Update drvsupport: flatgeobuf gained support to append with gdal 3.5, openfilegdb will gain support for write and append with gdal 3.6
The tests testing GDAL functionality (e.g. read / write / append support of drivers or support of drivers for time / datetime) are now marked with @pytest.mark.gdal. These tests should only be executed when adding the --test_gdal
flag to pytest.
Thanks @mwtoews for having a look at this PR.
There are still some regressions:
The cmake configuration step emits the following warning and I'm not exactly sure why:
CMake Warning:
Ignoring extra path from command line:
"/"
The FileGDB driver is not included with the cmake install script. But as gdal 3.6 seems to support writing GDB files with the OpenFileGDB driver this seems not be that important.
@rbuffat can we do without the --test_gdal
option? Is there anything about it that isn't possible with pytest -m gdal"
or pytest -m "not gdal"
?
@sgillies Sorry I forgot about this PR.
@rbuffat can we do without the --test_gdal option? Is there anything about it that isn't possible with pytest -m gdal" or pytest -m "not gdal"?
What I intended to implement is that the gdal
marked tests are disabled by default and must be enabled manually. So that we can enable it on our CI and we can avoid situations where others have issues with them when they fail on exotic platforms. Based on https://stackoverflow.com/questions/66315234/exclude-some-tests-by-default it should work also without an additional command line argument. Lets see if this works.
That seems not to work as intended. I will have a look at it later.
With the current changes, when pytest is run using pytest -m "not wheel" ...
or pytest ...
(not explicitly including the gdal
marker) all tests marked with @pytest.mark.gdal
are skipped:
tests/test_datetime.py ...sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss.
tests/test_drvsupport.py ssssssssssssssssssssssssssssssssssssssssssssssssss.
When using pytest -m "not wheel or gdal"
(explicitly including the gdal marker), these tests are not skipped and are included:
tests/test_datetime.py .................................................................................................
tests/test_drvsupport.py ...............................x...................
@sgillies I'm not sure this is the way this should be done. It looks too complicated, which often indicates that something is done wrong. I will look at it again when I have more brain capacity left. Or what is your opinion, should we avoid meddling with the markers in conftest.py/pytest_collection_modifyitems
?
@sgillies This PR should now be ready for a review.
- If you don't like the changes in conftest.py/pytest_collection_modifyitems, let's just remove them.
- pytest.mark.gdal is not the best name, maybe pytest.mark.ci_only would be better?
@rbuffat, friendly ping. Is this something you see moving forward soon?
@zklaus The PR is ready to be reviewed. It is currently only for the 1.9 branch, but the same changes should also apply for the 1.8 branch.
This PR does not change the functionality of Fiona (expect that without it Fiona limits some driver capabilities that GDAL gained with version 3.6)
@sgillies should this be backported to maint-1.8 for a 1.8.23 release?
@rbuffat if people use Fiona 1.8.22 with 3.6.0 they will see a few failing tests, but no breakage, right? I'm inclined to not backport and not make a new release.
Fyi, I have effectively backported this in conda-forge/fiona-feedstock#203