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

Add Mac and Windows to CI #304

Merged
merged 61 commits into from
Oct 14, 2022
Merged

Conversation

rhugonnet
Copy link
Contributor

@rhugonnet rhugonnet commented Sep 18, 2022

Summary

This PR adds Windows and Mac to the full test suite run during CI. We now have 9 jobs running in CI (Python 3.8, 3.9 and 3.10 for Ubuntu, Windows and Mac).
All core functionalities are working, however a couple little issues remain, opened for later work: making outputs of SciPy fully consistent between OS due to floating point considerations, making Sphinx work on all OS, and making PROJ work on Windows to use proj-data grids.

In details

This PR:

  • Unsets the PROJ_LIB variable during CI to allow rasterio to import correctly, otherwise failing on Windows. This fix is recommended by https://rasterio.readthedocs.io/en/latest/faq.html#why-can-t-rasterio-find-proj-db-rasterio-from-pypi-versions-1-2-0. Curiously, this is not needed in GeoUtils' CI, I couldn't figure why it fails in xDEM exactly (similar environments);
  • Performs rounding of the results of scipy.optimize.least_squares after setting a lower xtol to ensure the optimized parameters found are the same between OSs. This works to get the exact same output of NuthKaab() co-registration on Windows and Linux, but somehow still differs on Mac... Added a comment on this in issue Fix results of scipy.optimize between OS #310.
  • Tries to do the same rounding operation for scipy.optimize.basin_hopping but the xtol parameter does not exist as such, we would need to derive the tolerance of the parameter from the scipy method and ftol or gtol parameters, opened issue Fix results of scipy.optimize between OS #310;
  • Skips tests related to grids stored by proj-data in test_dem.py on Windows, that currently can't be found, opened issue PROJ grids not found on Windows #311;
  • Skips the building of the documentation with Sphinx that fails on both Mac and Windows with the same TypeError: 'module' object is not callable when xdem.examples.get_path() is called, notably in examples/*/plot_*.py scripts for sphinx-gallery, opened Sphinx fails when building on Mac or Windows #316.

Unrelated but needed to fix bugs related to package updates, this PR:

Resolves #126

@rhugonnet rhugonnet marked this pull request as draft September 18, 2022 18:50
@adehecq
Copy link
Member

adehecq commented Sep 20, 2022

Haha, I see some failure on Windows 😆

@rhugonnet rhugonnet marked this pull request as ready for review October 13, 2022 20:40
@rhugonnet
Copy link
Contributor Author

One issue closed, four open! But at least now we know that all the important stuff works as intended on all OSs! 🥳

Ready to merge, then I'll add the cache in xDEM to speed up CI, that can take up to 30min right now...

adehecq
adehecq previously approved these changes Oct 14, 2022
Copy link
Member

@adehecq adehecq left a comment

Choose a reason for hiding this comment

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

Looking good !
CI will take even longer now, but hopefully the cache will speed things up.

@rhugonnet
Copy link
Contributor Author

rhugonnet commented Oct 14, 2022

Merging as there's only two comments added in the code since the last approval

@rhugonnet rhugonnet merged commit 02d5e44 into GlacioHack:main Oct 14, 2022
@rhugonnet rhugonnet deleted the ci_mac_windows branch October 14, 2022 16:13
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.

Add Windows CI with GitHub
2 participants