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

Attempt to fix doctests by using airports layer in epsg4326 instead of epsg2964. #9256

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

folinimarc
Copy link
Contributor

@folinimarc folinimarc commented Sep 12, 2024

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

  • Use the airports dataset from data/data.gpkg (EPSG:4326) instead of the Alaska scoped airports.shp (EPSG:2964) and remove the warning string as expected test output. We do not simply reproject the layer, because the Alaska airports crs is mentioned multiple times in the cookbook and streamlining the whole cookbook is out of scope for this PR.

Secondary changes

  • Prefer usage of geopackage instead of shapefile in the spirit of slowly fading out .shp hopefully.
  • Minor refactoring of documentation along the way.

…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.
@folinimarc
Copy link
Contributor Author

folinimarc commented Sep 13, 2024

@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!

@folinimarc
Copy link
Contributor Author

folinimarc commented Sep 13, 2024

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:

Doctest summary
===============
  330 tests
    0 failures in tests
    0 failures in setup code
    0 failures in cleanup code
rediraffe: Redirect generation skipped for unsupported builders. Supported builders: html, dirhtml, readthedocs, readthedocsdirhtml.
build succeeded.

Testing of doctests in the sources finished, look at the results in build/doctest/output.txt.
Segmentation fault (core dumped)
make: *** [Makefile:131: doctest-gh] Error 139
make: *** [docker.mk:5: doctest] Error 2
Error: Process completed with exit code 2.

@folinimarc
Copy link
Contributor Author

folinimarc commented Sep 13, 2024

@selmaVH1 Good news, it might be that the segfault was just one time bad luck. I triggered the workflow multiple times now and it consistently passes (pass, pass, pass, pass).

I merged master into the branch and I think it would be worth to review this PR and give it a shot?

@selmaVH1
Copy link
Collaborator

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Flaky doctests due to "Using non-preferred coordinate operation between EPSG:2964 and EPSG:4326."
2 participants