Skip to content

test: Add predict and predict_proba tests for GWLogisticRegression#116

Open
jigyasaba wants to merge 13 commits into
pysal:mainfrom
jigyasaba:pr-predict-tests
Open

test: Add predict and predict_proba tests for GWLogisticRegression#116
jigyasaba wants to merge 13 commits into
pysal:mainfrom
jigyasaba:pr-predict-tests

Conversation

@jigyasaba

Copy link
Copy Markdown
Contributor

Summary:

Adds basic test coverage for predict and predict_proba in GWLogisticRegression

Changes:

  • Added test for predict
  • checks output length
  • validates binary predictions (ignoring missing values)
  • Added test for predict_proba
  • checks output shape (n_samples, 2)
  • Added test for keep_models=False
  • ensures prediction fails when local models are not stored

Notes:

  • geometry is required for prediction and is included in tests
  • Some predictions may be NaN, which is handled in assertions

Testing:

  • All tests pass locally (pytest)
  • Code formatted with ruff
  • Checklist

@jigyasaba jigyasaba changed the title test: Add predict and predict_proba tests for GWLogisticRegressionPr predict tests test: Add predict and predict_proba tests for GWLogisticRegression Apr 8, 2026
@codecov

codecov Bot commented Apr 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.35%. Comparing base (a687b09) to head (dcd2b56).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #116   +/-   ##
=======================================
  Coverage   92.35%   92.35%           
=======================================
  Files           6        6           
  Lines         876      876           
=======================================
  Hits          809      809           
  Misses         67       67           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jigyasaba

Copy link
Copy Markdown
Contributor Author

pre-commit.ci autofix

@martinfleis martinfleis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks, though I am not sure this is needed. We test both predict and predict_proba on base models and subclasses do not alter that code in any way. I would rather try to find a way how to trim our test suite to make it quicker, than adding more models to be trained.

@jigyasaba

Copy link
Copy Markdown
Contributor Author

You're right that predict and predict_proba are already tested at the base model level.
My intention here was to ensure that the prediction workflow works correctly when used through GWLogisticRegression, particularly with spatial inputs like geometry and the keep_models setting.

That said, I understand the concern about test suite runtime and redundancy. I'm happy to reduce or simplify these tests
Please let me know your preference, and I can update the PR accordingly.

@martinfleis

Copy link
Copy Markdown
Member

I would just not do anything as these features are already covered by existing tests.

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