Skip to content

fix: support mixed bool and float in maxp#519

Open
samay2504 wants to merge 1 commit into
pysal:mainfrom
samay2504:fix/maxp-mixed-covariates
Open

fix: support mixed bool and float in maxp#519
samay2504 wants to merge 1 commit into
pysal:mainfrom
samay2504:fix/maxp-mixed-covariates

Conversation

@samay2504
Copy link
Copy Markdown

This PR fixes issue #498 by normalising MaxP covariate input to a numeric array before distance computation, which allows mixed bool and float columns to be used safely without triggering SciPy object-dtype errors, and it adds a regression test that builds a mixed bool+float attribute set and verifies MaxPHeuristic.solve() completes and returns labels for all observations.

Copilot AI review requested due to automatic review settings April 11, 2026 17:09
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes MaxPHeuristic failing when attrs_name contains mixed bool + numeric columns by ensuring the attribute matrix passed into SciPy distance computation is always numeric, and adds a regression test for the reported scenario (issue #498).

Changes:

  • Cast gdf[attrs_name] to a float NumPy array prior to pdist to avoid object-dtype failures when boolean columns are included.
  • Add a regression test that constructs a mixed bool+float attribute set and asserts solve() returns labels for all observations.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
spopt/region/maxp.py Normalizes covariate inputs to a numeric (float) array before computing distances.
spopt/tests/test_region/test_maxp.py Adds regression coverage for mixed boolean + float covariates in MaxPHeuristic.solve().

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 11, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.9%. Comparing base (9b36063) to head (fb90ff5).
⚠️ Report is 7 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main    #519   +/-   ##
=====================================
  Coverage   76.9%   76.9%           
=====================================
  Files         29      29           
  Lines       3700    3700           
=====================================
  Hits        2846    2846           
  Misses       854     854           
Files with missing lines Coverage Δ
spopt/region/maxp.py 98.0% <100.0%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

2 participants