-
Notifications
You must be signed in to change notification settings - Fork 117
Mark tests that require network or geotiff dependency. #698
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
base: main
Are you sure you want to change the base?
Conversation
a193bcd to
27edcf1
Compare
| self.assertTrue(validateshapefile(shapefile_input, MODE.STRICT), 'STRICT validation') | ||
| shapefile_input.stream.close() | ||
|
|
||
| @pytest.mark.geotiff |
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.
Why not simply add the geotiff library to the test requirements?
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.
Why not simply add the
geotifflibrary to the test requirements?
Because the library is not packaged, as mentioned in the OP:
The geotiff module is also not available.
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.
I'm not sure what you mean.
PyPI: https://pypi.org/project/geotiff/
Conda: https://github.com/conda-forge/python-geotiff-feedstock
geotiff is a core dependency as well:
Line 4 in 04f734b
| geotiff |
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.
I'm not sure what you mean.
There is no Debian package for the geotiff Python library:
$ apt-file search dist-packages/geotiff ; echo $?
1
See also: https://lists.debian.org/debian-gis/2025/12/msg00011.html
geotiffis a core dependency as well:
It is not imported unconditionally:
https://github.com/geopython/pywps/blob/main/pywps/validator/complexvalidator.py#L332-L338
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.
Ah, this is specifically a Debian issue! My bad.
If we want to have this level of change, I would suggest that we remove geotiff from the core dependencies and add it to an extras for tracking optional dependencies.
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.
@sebastic @Zeitsperre @ldesousa can we limit support to conda-forge? I'm not sure if anyone is using direct installation on debian based dists.
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.
If core dependencies are adjusted to remove format-specific libraries, that will help reduce the package, which is great since not every deployment employs all combinations of data formats for their processing.
However, if this change is done, it should be marked as a higher release than a simple patch, since deployments relying on them could break.
Another thing to clarify in the release notes would be that these libraries are actually required only if using ">=STRICT" validation mode (they are imported under them in https://github.com/geopython/pywps/blob/main/pywps/validator/complexvalidator.py), so the formats could still be employed in I/O definitions with lower validation if the libraries are not installed.
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.
@sebastic @Zeitsperre @ldesousa can we limit support to conda-forge? I'm not sure if anyone is using direct installation on debian based dists.
OSGeo-Live is using the Debian package currently:
https://github.com/OSGeo/OSGeoLive/blob/master/bin/install_pywps.sh#L53
That can of course be changed, I can then stop spending time on the pywps package in Debian.
CC: @kalxas
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.
zarr v3 support has been notoriously difficult to adopt due to some very breaking changes. Most projects I work on are pinned below that version. geotiff would need to address this issue before anything.
Who is "us"?
I've worked on geotiff as well and could try my hand at packaging it for Debian (for personal learning purposes). There are other packages (non-Python) I work on that I'd love to see in Debian/Ubuntu.
Yes, please. Treat fiona, geotiff, and netCDF4 in the same way.
I can justify putting some work towards this for this week. Fun end of year project, maybe. @sebastic Would you be fine with me cherry-picking your commit here?
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.
Who is "us"?
I've worked on
geotiffas well and could try my hand at packaging it for Debian (for personal learning purposes). There are other packages (non-Python) I work on that I'd love to see in Debian/Ubuntu.
You're very welcome to join the Debian GIS team:
https://debian-gis-team.pages.debian.net/policy/index.html#membership
Yes, please. Treat fiona, geotiff, and netCDF4 in the same way.
I can justify putting some work towards this for this week. Fun end of year project, maybe. @sebastic Would you be fine with me cherry-picking your commit here?
Sure, go ahead.
Overview
Add pytest markers to enable skipping certain tests like those that require network or certain dependencies.
Additional Information
The Debian package build environment doesn't have network, tests requiring network need to be skipped.
The geotiff module is also not available.
Contribution Agreement