paleolimbot / s2geography

Simple features (ish) for s2geometry
Other
29 stars 3 forks source link

Update CI: run tests with conda installed dependencies #4

Closed benbovy closed 1 year ago

benbovy commented 1 year ago

This PR adds a github workflow to build and run the tests on Linux / MacOS / Windows after installing dependencies with conda.

codecov-commenter commented 1 year ago

Codecov Report

Base: 14.36% // Head: 14.36% // No change to project coverage :thumbsup:

Coverage data is based on head (22f07b2) compared to base (95f0af7). Patch has no changes to coverable lines.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #4 +/- ## ======================================= Coverage 14.36% 14.36% ======================================= Files 14 14 Lines 1490 1490 Branches 29 29 ======================================= Hits 214 214 Misses 1270 1270 Partials 6 6 ``` Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Dewey+Dunnington). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Dewey+Dunnington)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

benbovy commented 1 year ago

Hmm not sure why we still have linking errors on Windows here. Another package that I maintain and that also depends on s2geometry has been built successfully on Windows with the last conda-forge s2 package: https://github.com/conda-forge/pys2index-feedstock/pull/12.

benbovy commented 1 year ago

Hmm not sure why we still have linking errors on Windows here.

Global data symbols are missing in the conda-forge s2geometry windows shared library (more info: https://cmake.org/cmake/help/latest/prop_tgt/WINDOWS_EXPORT_ALL_SYMBOLS.html). We'll probably need a new s2geometry conda-forge build with one more patch to manually add __declspec(dllexport) to the missing symbols.

paleolimbot commented 1 year ago

Sorry I haven't been of more help here! (The R windows package uses msys2/mingw/gcc, so I haven't run into these particular linking issues before).

benbovy commented 1 year ago

No worries! https://github.com/conda-forge/s2geometry-feedstock/pull/10 will hopefully fix it.

benbovy commented 1 year ago

Windows / MSVC gave me some headache here but hopefully this will be helpful later, e.g., for packaging s2geography on conda-forge.

The conda environment cache might be a bit overkill, I just wanted to try it (following steps from the setup-miniconda action documentation). It'll save some package downloads anyway.