Matching

[ljwolf]

Hello! Please make sure to check all these boxes before submitting a Pull Request (PR). Once you have checked the boxes, feel free to remove all text except the justification in point 5.

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/main branch.
3. [x] This pull introduces new functionality covered by docstrings and unittests?
4. [x] You have assigned a reviewer and added relevant labels
5. [x] The justification for this PR is contained in #665

todo:

• [x] add pulp to our testing matrix.
• [x] write tests using the toy example in the gist
• [x] write an example notebook

[codecov[bot]]

Codecov Report

Attention: 15 lines in your changes are missing coverage. Please review.

Comparison is base (e24adfb) 85.0% compared to head (a24c979) 85.0%. Report is 18 commits behind head on main.

Additional details and impacted files

[](https://app.codecov.io/gh/pysal/libpysal/pull/666?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pysal) ```diff @@ Coverage Diff @@ ## main #666 +/- ## ======================================= - Coverage 85.0% 85.0% -0.0% ======================================= Files 139 141 +2 Lines 14889 15023 +134 ======================================= + Hits 12663 12771 +108 - Misses 2226 2252 +26 ``` | [Files](https://app.codecov.io/gh/pysal/libpysal/pull/666?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pysal) | Coverage Δ | | |---|---|---| | [libpysal/graph/base.py](https://app.codecov.io/gh/pysal/libpysal/pull/666?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pysal#diff-bGlicHlzYWwvZ3JhcGgvYmFzZS5weQ==) | `97.8% <100.0%> (+0.1%)` | :arrow_up: | | [libpysal/graph/tests/test\_builders.py](https://app.codecov.io/gh/pysal/libpysal/pull/666?src=pr&el=tree&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\_matching.py](https://app.codecov.io/gh/pysal/libpysal/pull/666?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pysal#diff-bGlicHlzYWwvZ3JhcGgvdGVzdHMvdGVzdF9tYXRjaGluZy5weQ==) | `97.9% <97.9%> (ø)` | | | [libpysal/graph/\_matching.py](https://app.codecov.io/gh/pysal/libpysal/pull/666?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pysal#diff-bGlicHlzYWwvZ3JhcGgvX21hdGNoaW5nLnB5) | `72.0% <72.0%> (ø)` | | ... and [3 files with indirect coverage changes](https://app.codecov.io/gh/pysal/libpysal/pull/666/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pysal)

[ljwolf]

Tests are now added, but pulp installation fails on windows through conda-forge. Is there a better way to do this?

[jGaboardi]

Tests are now added, but pulp installation fails on windows through conda-forge. Is there a better way to do this?

In spopt we install pulp via pip --> https://github.com/pysal/spopt/blob/4760bd9e03f9b31c003bfa7f5d785a2a81b174bd/ci/312-latest.yaml#L32

[jGaboardi]

Looks like numpy is never being imported --> https://github.com/pysal/libpysal/actions/runs/7159612453/job/19493044782?pr=666#step:7:2098

[jGaboardi]

Is this passing locally for you?

[knaaptime]

i think it just needs to be wrapped in a tuple

[ljwolf]

Yes, but fixing that, I still have fails locally. Will fix.

[ljwolf]

One question I had for maintainers:

I built the spatial matching framework to be able to do cross-pattern queries. So, something like:

For all points in pattern X, find at least k matched points in pattern Y such that the total distance of X->Y matches are minimised.

I think this would be good to expose in cg, since it's a general purpose spatial matching function for computational geometry applications. I have something even more general that will probably make it into a separate package for spatial causal inference in python, but the spatial implementation can live here.

So, would it be OK to grab _spatial_matching() from libpysal.graph._matching and expose it directly in libpysal.cg? I could alternatively define a dispatching function that requires the second pattern (y argument) to be provided.

[ljwolf]

The code is ready to review. I'm not sure why the ruff check is failing... I've run the ruff auto formatter myself locally on the submission (2c04acf, 215022d, 273e6ea)

[jGaboardi]

The code is ready to review. I'm not sure why the ruff check is failing... I've run the ruff auto formatter myself locally on the submission (2c04acf, 215022d, 273e6ea)

Do you have the most recent ruff installed locally? I think there was a fresh version released last night. Could be playing a part.

[ljwolf]

Yes, not quite sure what's up here:

0.1.8 is the most recent version of ruff, and it fixes nothing both in the CI and locally?

[martinfleis]

It is ruff-format that is failing, not ruff linter.

[ljwolf]

OK! Wasn't clear on the differences between ruff check and format. Sorry!

I think this is complete.

[jGaboardi]

The large diff in base is just moving stuff around to get alphabetic order of builders?

Yes.

[ljwolf]

I believe I've addressed all comments!

[ljwolf]

Hi! could one of the approvers please merge this?

[ljwolf]

thank you!