Skip to content

Add real pytest tests and CI linting (black, ruff, mypy)#8

Merged
dantzert merged 3 commits intomainfrom
copilot/add-pytest-and-linting
Feb 28, 2026
Merged

Add real pytest tests and CI linting (black, ruff, mypy)#8
dantzert merged 3 commits intomainfrom
copilot/add-pytest-and-linting

Conversation

Copy link
Contributor

Copilot AI commented Feb 28, 2026

The existing "test" files were standalone scripts with no assertions or pytest structure — nothing discoverable or verifiable by a test runner. This PR converts the project to proper pytest and wires up a CI pipeline with linting.

Changes

modpods.py

  • Fixed infer_causative_topology: total_graph was initialized as object dtype, breaking nx.from_pandas_adjacency on newer NetworkX. Added dtype=float to the DataFrame constructor.
  • Fixed transform_inputs for pandas 3.x Copy-on-Write: column accumulation now uses a numpy array before assignment.
  • Fixed ps.SINDy() calls: removed feature_names parameter (dropped in pysindy 2.x).
  • Fixed ConstrainedSR3 usage: imported from pysindy.optimizers._constrained_sr3; updated parameter names (thresholdreg_weight_lam, thresholderregularizer, nurelax_coeff_nu).
  • Fixed library.fit() in lti_system_gen: replaced AxesArray wrapping a Python list with a plain np.zeros array.
  • Fixed constraint_rhs = 0 scalar → np.zeros(1) array for pysindy 2.x compatibility.
  • Fixed A/B/C matrix finalization in lti_system_gen: replaced fillna(0) (which ignores non-NaN string values) with pd.to_numeric(errors='coerce').fillna(0).
  • Fixed topo_from_pystorms: resets pyswmm 2.x single-simulation guard before opening its own connection.
  • Fixed np.NANnp.nan throughout (removed in numpy 2.x).

tests/test_modpods.py

Expanded from 8 to 18 pytest tests covering all original test scripts (now deleted):

Tests Source script Mark
lti_from_gamma (4) test_lti_from_gamma.py
delay_io_train + delay_io_predict (4) test_coef_constraints.py
infer_causative_topology (4) test_topo_inference.py
lti_system_gen (1) test_lti_system_gen.py @pytest.mark.slow
topo_from_pystorms zeta (2) test_topo_from_swmm.py @pytest.mark.slow
topo_from_pystorms gamma (1) test_lti_control_of_swmm.py @pytest.mark.slow
CAMELS train + predict (2) test.py @pytest.mark.slow

All 18 tests run in CI (slow tests are marked but not skipped).

tests/data/

Large data files tracked in version control moved here from the repo root:

  • 03439000_05_model_output.txt (1.4 MB CAMELS daily streamflow data)
  • swmm_lti_plant_approx_seeing.pickle (53 KB trained SWMM plant model)

Old test scripts deleted

test.py, test_coef_constraints.py, test_lti_from_gamma.py, test_lti_system_gen.py, test_topo_from_swmm.py, test_lti_control_of_swmm.py, test_topo_inference.py

Infrastructure

  • requirements.txt — updated to pysindy≥2.0; added cvxpy, pystorms, dill
  • pyproject.toml — black, ruff (E/F/W/I, no E501), mypy (lenient for untyped codebase), pytest testpaths, slow marker registered
  • .github/workflows/ci.yml — runs black --check tests/, ruff check tests/, mypy tests/ modpods.py, then pytest tests/ -v on every push/PR; scoped with permissions: contents: read
Original prompt

create a branch called "real-tests" and make the tests in this repo actually rely on pytest. also add linting through black, ruff, and mypy to the CICD workflow.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@dantzert
Copy link
Owner

This merge would close #2

…black/ruff/mypy

Co-authored-by: dantzert <47285626+dantzert@users.noreply.github.com>
Copilot AI changed the title [WIP] Add pytest and linting to CI/CD workflow Add real pytest tests and CI linting (black, ruff, mypy) Feb 28, 2026
@dantzert dantzert requested review from Copilot and removed request for Copilot February 28, 2026 14:22
Comment on lines 4 to 5
These tests cover the functions that are self-contained and do not require
external data files or long-running simulations.
Copy link
Owner

Choose a reason for hiding this comment

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

The tests should include the larger data files that are tracked in version control as well. Move any larger files to tests/data and give them a pytest slow mark, but don't skip them in CI.

Further, those old testing files should be deleted once all their functions are collected

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in commit a1705b9. Here's what was addressed:

  • All 7 old test scripts deleted — functions collected from: test.py, test_coef_constraints.py, test_lti_from_gamma.py, test_lti_system_gen.py, test_topo_from_swmm.py, test_lti_control_of_swmm.py, test_topo_inference.py
  • Large data files moved to tests/data/03439000_05_model_output.txt (1.4 MB) and swmm_lti_plant_approx_seeing.pickle (53 KB)
  • @pytest.mark.slow applied to tests that load those files or run long pystorms/training simulations (5 tests); the CI workflow runs all tests including slow ones
  • slow marker registered in pyproject.toml

The conversion also required fixing several upstream compatibility issues in modpods.py to make the tests pass against current deps (pandas 3.x CoW, pysindy 2.x API, numpy 2.x, pyswmm 2.x multi-simulation guard), so all 18 tests now pass in ~2.5 minutes.

@dantzert
Copy link
Owner

@copilot please address the feedback in my latest code review and send a notification once you've passed CI and are ready for my review

… dependency compat

Co-authored-by: dantzert <47285626+dantzert@users.noreply.github.com>
@dantzert dantzert marked this pull request as ready for review February 28, 2026 20:17
@dantzert dantzert merged commit 8532d8e into main Feb 28, 2026
2 checks passed
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