Skip to content

Conversation

dbaston
Copy link
Member

@dbaston dbaston commented Sep 10, 2025

What does this PR do?

Enables CI testing of Python API examples, using pytest's built-in doctest integration.

To avoid polluting the examples with a bunch of setup code or complicated asserts, it relies on a two hacks in conftest.py.

Hack 1: We break into the internals of pytest to add custom result matching code. This lets us use human-readable comments in the expected output to control how the matching is done. Some situations where this is needed:

  • Content written by GDAL to stdout is not detected by doctest and therefore causes output matching to fail. Keeping these outputs in the documentation is important in cases like Feature.DumpReadable. The # no-check comment causes the matcher to ignore these.
  • Some function results may be quite different depending on the particulars of the PROJ install. pytest allows fuzzy floating-point matching but provides no mechanism for non-exact matching of digits to the left of the decimal point. The # rtol: 1e-4 syntax allows this.
  • Some function results cannot be reproduced but we don't want to skip the test entirely; we still want to test that the code executes but and produces an output of the correct type and shape. The # random syntax allows this.

The code that handles these comments may need to be made more robust in the future, but it's good enough for the API examples we have so far.

pytest-doctestplus https://github.com/scientific-python/pytest-doctestplus and https://github.com/scipy/scipy_doctest both make some efforts along the same lines but neither one covers all of these cases, and I think the hack is preferable to contorting the examples.

Hack 2: To avoid including lengthy paths to input files (byte.tif, not ../../autotest/gcore/data/byte.tif), we keep a list of the input test files and their locations, then monkey-patch GDAL functions that might try to access these files (gdal.Open, etc.) and do a find-and-replace of their arguments to substitute the full paths. The alternative would be to use the .. testsetup:: and .. testcleanup:: directives to modify working directories before and after each test, but this seems tedious.

This PR adds a 169kb test data file. If there's already a file in the repo that uses a RAT I could rewrite the RAT tests to avoid that. I could also make a new, smaller subset of the same data.

What are related issues/pull requests?

#9949

Tasklist

  • Review
  • Adjust for comments
  • All CI builds and checks have passed

@dbaston dbaston added python bindings documentation Issues and contributions to the documentation content test suite labels Sep 10, 2025
@rouault
Copy link
Member

rouault commented Sep 10, 2025

Looks very nice !

It would be good to have a section in doc/source/development/dev_documentation.rst to explain (e.g. extracting examples of this PR) the new syntax element, especially the GDAL specific tricks.

This PR adds a 169kb test data file. If there's already a file in the repo that uses a RAT I could rewrite the RAT tests to avoid that. I could also make a new, smaller subset of the same data.

We have autotest/gcore/data/gtiff/testrat.tif

@coveralls
Copy link
Collaborator

coveralls commented Sep 10, 2025

Coverage Status

coverage: 71.213% (-0.003%) from 71.216%
when pulling d227192 on dbaston:pytest-doctest
into bf52089 on OSGeo:master.

###

files = {
"byte.tif": "autotest/gcore/data/byte.tif",
Copy link
Member

@rouault rouault Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is an error on Windows about not finding the "autotest/..." files. I believe that the current directory from which this is run must be the build directory. On Unix, there are symbolic links for convenience for $src_dir/autotest/{subdir} to $build_dir/autotest/{subdir}, but not on Windows.

So safer would be to have a mapping like

files = {
"byte.tif": abs_path_to("autotest/gcore/data/byte.tif"),
...
}

with abs_path_to that prepends the path of __file__ / .. / ..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Issues and contributions to the documentation content python bindings test suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants