search

pysal / libpysal

Core components of Python Spatial Analysis Library
http://pysal.org/libpysal
Other
249 stars 79 forks source link

Use fixtures for test data that uses the network #726

Closed QuLogic closed 1 week ago

QuLogic commented 1 week ago
  1. [x] You have run tests on this submission locally using pytest on your changes. Continuous integration will be run on all PRs with GitHub Actions, but it is good practice to test changes locally prior to a making a PR.
  2. [x] This pull request is directed to the pysal/master branch. This should say main now.
  3. [x] This pull introduces new functionality covered by docstrings and unittests?
  4. [n/a] You have assigned a reviewer and added relevant labels
  5. [x] The justification for this PR is:

Currently, if you have no network, the marked tests will fail. However, much worse than that, because some datasets are loaded at the top level, pytest collection will ERROR out. This makes it impossible to even run the tests that don't use the network.

Moving these datasets from the top level to fixtures means that pytest collection works, and those tests can be skipped by deselecting the network marker.

codecov[bot] commented 1 week ago

Codecov Report

Attention: Patch coverage is 99.09910% with 1 line in your changes missing coverage. Please review.

Project coverage is 85.2%. Comparing base (e82d1be) to head (df2197c). Report is 5 commits behind head on main.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/pysal/libpysal/pull/726/graphs/tree.svg?width=650&height=150&src=pr&token=wgnkG5Rj0J&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pysal)](https://app.codecov.io/gh/pysal/libpysal/pull/726?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pysal) ```diff @@ Coverage Diff @@ ## main #726 +/- ## ======================================= + Coverage 83.9% 85.2% +1.2% ======================================= Files 145 145 Lines 15583 15639 +56 ======================================= + Hits 13077 13318 +241 + Misses 2506 2321 -185 ``` | [Files](https://app.codecov.io/gh/pysal/libpysal/pull/726?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pysal) | Coverage Δ | | |---|---|---| | [libpysal/graph/tests/test\_base.py](https://app.codecov.io/gh/pysal/libpysal/pull/726?src=pr&el=tree&filepath=libpysal%2Fgraph%2Ftests%2Ftest_base.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pysal#diff-bGlicHlzYWwvZ3JhcGgvdGVzdHMvdGVzdF9iYXNlLnB5) | `100.0% <100.0%> (ø)` | | | [libpysal/graph/tests/test\_builders.py](https://app.codecov.io/gh/pysal/libpysal/pull/726?src=pr&el=tree&filepath=libpysal%2Fgraph%2Ftests%2Ftest_builders.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pysal#diff-bGlicHlzYWwvZ3JhcGgvdGVzdHMvdGVzdF9idWlsZGVycy5weQ==) | `100.0% <100.0%> (ø)` | | | [libpysal/graph/tests/test\_contiguity.py](https://app.codecov.io/gh/pysal/libpysal/pull/726?src=pr&el=tree&filepath=libpysal%2Fgraph%2Ftests%2Ftest_contiguity.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pysal#diff-bGlicHlzYWwvZ3JhcGgvdGVzdHMvdGVzdF9jb250aWd1aXR5LnB5) | `100.0% <100.0%> (ø)` | | | [libpysal/graph/tests/test\_kernel.py](https://app.codecov.io/gh/pysal/libpysal/pull/726?src=pr&el=tree&filepath=libpysal%2Fgraph%2Ftests%2Ftest_kernel.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pysal#diff-bGlicHlzYWwvZ3JhcGgvdGVzdHMvdGVzdF9rZXJuZWwucHk=) | `99.0% <100.0%> (+0.1%)` | :arrow_up: | | [libpysal/graph/tests/test\_matching.py](https://app.codecov.io/gh/pysal/libpysal/pull/726?src=pr&el=tree&filepath=libpysal%2Fgraph%2Ftests%2Ftest_matching.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pysal#diff-bGlicHlzYWwvZ3JhcGgvdGVzdHMvdGVzdF9tYXRjaGluZy5weQ==) | `98.0% <100.0%> (+0.1%)` | :arrow_up: | | [libpysal/graph/tests/test\_plotting.py](https://app.codecov.io/gh/pysal/libpysal/pull/726?src=pr&el=tree&filepath=libpysal%2Fgraph%2Ftests%2Ftest_plotting.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pysal#diff-bGlicHlzYWwvZ3JhcGgvdGVzdHMvdGVzdF9wbG90dGluZy5weQ==) | `100.0% <100.0%> (ø)` | | | [libpysal/graph/tests/test\_set\_ops.py](https://app.codecov.io/gh/pysal/libpysal/pull/726?src=pr&el=tree&filepath=libpysal%2Fgraph%2Ftests%2Ftest_set_ops.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pysal#diff-bGlicHlzYWwvZ3JhcGgvdGVzdHMvdGVzdF9zZXRfb3BzLnB5) | `100.0% <100.0%> (ø)` | | | [libpysal/graph/tests/test\_triangulation.py](https://app.codecov.io/gh/pysal/libpysal/pull/726?src=pr&el=tree&filepath=libpysal%2Fgraph%2Ftests%2Ftest_triangulation.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pysal#diff-bGlicHlzYWwvZ3JhcGgvdGVzdHMvdGVzdF90cmlhbmd1bGF0aW9uLnB5) | `98.8% <100.0%> (+<0.1%)` | :arrow_up: | | [libpysal/weights/tests/test\_contiguity.py](https://app.codecov.io/gh/pysal/libpysal/pull/726?src=pr&el=tree&filepath=libpysal%2Fweights%2Ftests%2Ftest_contiguity.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pysal#diff-bGlicHlzYWwvd2VpZ2h0cy90ZXN0cy90ZXN0X2NvbnRpZ3VpdHkucHk=) | `100.0% <100.0%> (ø)` | | | [libpysal/graph/tests/test\_utils.py](https://app.codecov.io/gh/pysal/libpysal/pull/726?src=pr&el=tree&filepath=libpysal%2Fgraph%2Ftests%2Ftest_utils.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pysal#diff-bGlicHlzYWwvZ3JhcGgvdGVzdHMvdGVzdF91dGlscy5weQ==) | `97.9% <96.2%> (-0.9%)` | :arrow_down: | ... and [7 files with indirect coverage changes](https://app.codecov.io/gh/pysal/libpysal/pull/726/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pysal)
QuLogic commented 1 week ago

On Linux, you can use unshare or systemd-run: https://unix.stackexchange.com/a/454767

martinfleis commented 1 week ago

@jGaboardi do we want to add another action testing this? It needs to be an action, not a job to avoid caching.

jGaboardi commented 1 week ago

@jGaboardi do we want to add another action testing this? It needs to be an action, not a job to avoid caching.

I'm not against that at all. Is it something we need in this PR? If not let merge. If so, I can try to put it on the To Do list for next month.

martinfleis commented 1 week ago

Is it something we need in this PR?

not at all. I am also not sure if we need it at all.