FIX: pulp regression issue by FirePheonix · Pull Request #907 · pysal/libpysal

FirePheonix · 2026-04-08T04:47:24Z

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.
This pull request is directed to the pysal/master branch.
This pull introduces new functionality covered by
docstrings and
unittests?
You have assigned a
reviewer and added relevant labels
The justification for this PR is:

This is a draft PR for testing if the PULP issue can be fixed with the following code change.
The error type object 'LpVariable' has no attribute 'dicts' indicates that the specific PuLP version or environment in CI is failing to expose the dicts helper method on the LpVariable class.

codecov · 2026-04-08T04:51:31Z

Codecov Report

❌ Patch coverage is 93.33333% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 85.7%. Comparing base (95829c9) to head (1826dbd).
⚠️ Report is 18 commits behind head on main.

Files with missing lines	Patch %	Lines
libpysal/graph/_matching.py	93.3%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##            main    #907     +/-   ##
=======================================
+ Coverage   85.6%   85.7%   +0.1%     
=======================================
  Files        151     151             
  Lines      16310   16342     +32     
=======================================
+ Hits       13967   14013     +46     
+ Misses      2343    2329     -14

Files with missing lines	Coverage Δ
libpysal/graph/_matching.py	`95.2% <93.3%> (+23.2%)`	⬆️

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

FirePheonix · 2026-04-08T05:24:16Z

@jGaboardi @martinfleis
I searched a lot on the internet, read some docs.
So, the CI failed because a pre-release of PuLP 4.0 was pulled, which features a massive internal rewrite in Rust.

As a result, older, explicit constructors like pulp.LpVariable.dicts and pulp.LpConstraint(...) were completely removed or modified, throwing errors.

I updated _matching.py to use a modern, version-agnostic approach:
(i haven't been able to find any other way out, i've tried a few times.)

Variables: We now dynamically check for and use mp.add_variable() for PuLP 4.x, gracefully falling back to pulp.LpVariable() for older versions.
Constraints: I replaced explicit LpConstraint(...) calls with Python's comparison operators (e.g., mp += summand >= n_matches).

This FORCES PuLP to build it's constraints automatically under the hood, regardless of the version installed.

ljwolf · 2026-04-08T05:30:25Z

Thanks for this--I wasn't aware of the PuLP 4.0 revision... we will need to adjust quite a bit in pysal/spopt as well. This offers a solid solution here.

In terms of strategy for libpysal, I think we should adopt the hasattr() check here, then pin the PuLP version after a few (2?) PySAL releases and remove the hasattr() check.

Thoughts?

FirePheonix · 2026-04-08T05:37:34Z

yea, sounds like a good move to me.
some projects in pysal might still be relying on PuLP 3.x internals.

we can pin to pulp >= 4.0 after a couple of release cycles once the v4 API officially stabilizes.

May I apply the same hasattr() fix for the spopt pulp fails too btw?

ljwolf · 2026-04-08T05:48:12Z

Yes, I think so! There's not a clear migration guide yet (just a quick start for the new version), but in spopt we use LpVariable (and .value()) all over the place. Both look deprecated in 4.0... So that will definitely be a bigger set of changes, and possibly a major release with a hard change to PuLP 4.0

jGaboardi · 2026-04-08T22:28:32Z

If we do this fix with via hasattr(), then we should leave a clear & concise comment in the code that is should be adjusted once pulp==v4 in the minimum. Otherwise, we should probably rely on packaging to provide version logic, like in graph._indices

I am much more in favor of doing this with explicit version checking.

For example, something like:

from packaging.version import Version
import pulp

PULP4 = Version(pulp.__version__) > Version("4")

if PULP4:
    # do `pulp==v4` logic
else:
    # do `pulp==v3` logic

This may be a bit more verbose, but it is explicit as to why we have the logic and makes eventual removal easy to determine. Same for in spopt.

FirePheonix · 2026-04-09T02:23:24Z

@jGaboardi
yeah.
I actually tried out using the hasattr() and refactoring the code in spopt on a side branch.

https://github.com/shubhammms/spopt/pull/1/changes
I managed to reduce the errors from 112 -> 6

The hasattr() actually also would use if/else, but the code gets wayyyy too confusing.

i like your suggestion on:

from packaging.version import Version
import pulp

PULP4 = Version(pulp.__version__) > Version("4")

if PULP4:
    # do `pulp==v4` logic

the if PULP4 makes it pretty clear instead of checking attributes every single time.
after the pulp 4 becomes stable, we might just be able to remove the else later on.

let's have a discussion on this on today's meet?

ljwolf · 2026-04-09T12:59:48Z

great idea @jGaboardi, I think that's a better approach. I wasn't aware!

martinfleis · 2026-04-09T15:30:12Z

    try:
        import pulp
    except ImportError as error:


Move the pulp4 = Version(pulp.__version__).major >= 4 here, these things are always on top after the imports for clarity. I would also make it a constant as per convention, so PULP_GE_4 (pulp greater or equal to 4). ~~Also, I am not sure how this evaluates for alpha release, it may not return True.~~

PULP_GE_4 with ruff ignoring -> done

but I don't think we should put pulp4 = Version(pulp.__version__).major >= 4 on top.
pulp is an optional dependency, we can't move the version check to module level- it would NameError on import for users without pulp installed.
Keeping it inside the function is intentional here.

But yes this suggestion is very much valid in spopt.

I meant to move it right after importing pulp. Not on top of the file.

aaaaah.
my bad. it's now done in the latest commit.

jGaboardi · 2026-04-11T22:30:42Z

-        indices=zip(row, col, strict=True),
-        cat="Continuous" if allow_partial_match else "Binary",
-    )
+    match_vars = {}


I'm curious about the need for a for-loop here. Is there no equivalent dicts functionality in pulp4 for batch variable addition?

LpVariable.dicts() names variables using the key directly in the string, so we'd get keys like "match_(0, 1)" instead of tuple keys (i, j).

And the rest of the code indexes match_vars[i, j] as a tuple.
Refactoring around that would add more complexity than the for-loop i think?
But let me if there's a cleaner way you have in mind..

and yeah, there IS addVariables() in pulp4, but it just takes already-created LpVariable objects, it's not a true batch creation equivalent.
The closest would still be LpVariable.dicts() + addVariables(), but that gives string keys like "match_(0,_1)" instead of the tuple keys (i, j)

I think we need to hold off on changing this unill they stabilize the 4.0 API.

I've just pulled down 4.0.0a3, which adds a LpProblem.add_variable_dicts() method, which does what we need it to do.

Let's wait until things stabilize with the 4.0 release, and move to the equivalent functionality then Let's keep this PR open for now, but it's very likely our implementation will not change this radically.

I second @ljwolf sentiment here and over in pysal/spopt#527. Well this ride here and allow dev CI to fail -- no harm, no foul.

@FirePheonix -- Let's mark this as Draft so it doesn't accidentally get merged. Same for pysal/spopt#527

jGaboardi · 2026-04-11T22:31:39Z

            )
            sense = int(not allow_partial_match)
-        mp += pulp.LpConstraint(summand, sense=sense, rhs=n_matches)
+        if sense == 1:


Same sentiment here as above. Did pulp4 really change the API that dramatically?

pulp-issue-test

8b4875a

FirePheonix added 2 commits April 8, 2026 10:33

fix(graph): support PuLP v4 API for variable creation

1db241b

fix(graph): support PuLP v4 API for constraints

665b9fd

FirePheonix marked this pull request as ready for review April 8, 2026 05:25

martinfleis approved these changes Apr 8, 2026

View reviewed changes

FirePheonix added 2 commits April 9, 2026 20:55

switch-from-attributes-to-pulp-version

333a3f7

lowercase-pulp4

799e058

martinfleis reviewed Apr 9, 2026

View reviewed changes

jGaboardi mentioned this pull request Apr 9, 2026

Bump actions/github-script from 8 to 9 #908

Merged

PULPv4-to-PULP_GE_4

6a3849a

FirePheonix requested a review from martinfleis April 11, 2026 05:21

move-PULP_GE_4

1826dbd

jGaboardi assigned FirePheonix Apr 11, 2026

jGaboardi requested a review from ljwolf April 11, 2026 20:46

FirePheonix mentioned this pull request Apr 11, 2026

Pulp v4 compatibility pysal/spopt#527

Open

jGaboardi reviewed Apr 11, 2026

View reviewed changes

jGaboardi mentioned this pull request May 9, 2026

TST: eliminate warning about unary_union #913

Merged

Conversation

FirePheonix commented Apr 8, 2026

Uh oh!

codecov Bot commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

FirePheonix commented Apr 8, 2026

Uh oh!

ljwolf commented Apr 8, 2026

Uh oh!

FirePheonix commented Apr 8, 2026

Uh oh!

ljwolf commented Apr 8, 2026

Uh oh!

jGaboardi commented Apr 8, 2026

Uh oh!

FirePheonix commented Apr 9, 2026

Uh oh!

ljwolf commented Apr 9, 2026

Uh oh!

martinfleis Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

FirePheonix Apr 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov Bot commented Apr 8, 2026 •

edited

Loading

martinfleis Apr 9, 2026 •

edited

Loading

FirePheonix Apr 11, 2026 •

edited

Loading