Skip to content

Conversation

@ShreyasN707
Copy link

Summary

This PR removes several noisy warnings (UserWarning + one DeprecationWarning) from tests/examples and adds regression tests to prevent them from returning. The changes improve determinism, clean test output, and stabilize CI.

Issue:#2904

Bug / Issue

The test suite emitted:

  • RNG warnings: Spaces (OrthogonalMooreGrid, Network, VoronoiGrid) created without an explicit RNG → non-deterministic tests.
  • PropertyLayer warnings: default_val=0 used with dtype=np.float64.
  • Portrayal warnings: Deprecated keys s, c, linecolors ignored by the matplotlib backend.
  • Deprecated import in test_components_matplotlib.py.
  • Playwright-dependent test failed when Playwright wasn't installed.

Implementation

  • Added seeded random.Random(42) to all grid/network/voronoi instantiations in tests/examples.
  • Updated PropertyLayer defaults to 0.0 or np.float64(0) when dtype is float.
  • Updated portrayal dicts to use size, color, edgecolors and added minimal normalization in the matplotlib backend.
  • Fixed deprecated import and added a skip guard for Playwright in test_examples_viz.py.

Testing

  • Added regression tests:
    • RNG determinism
    • PropertyLayer dtype
    • Portrayal key normalization
    • No-RNG-warning check
  • Verified pytest -q passes and warnings are gone.
  • Playwright tests now skip gracefully if not installed.
  • Pre-commit and Codecov thresholds remain satisfied.

Additional Notes

  • No public API behavior was changed.
  • Normalization helper is temporary and isolated.
  • Changes are split into small commits for safe review.

@github-actions
Copy link

github-actions bot commented Dec 1, 2025

Performance benchmarks:

Model Size Init time [95% CI] Run time [95% CI]
BoltzmannWealth small 🔵 +2.3% [+1.2%, +3.3%] 🔵 +1.7% [+1.5%, +1.8%]
BoltzmannWealth large 🔵 -0.0% [-0.4%, +0.4%] 🔵 +1.6% [+0.7%, +2.4%]
Schelling small 🔵 -0.9% [-1.0%, -0.7%] 🔵 -1.3% [-1.5%, -1.2%]
Schelling large 🔵 -1.0% [-1.4%, -0.6%] 🔵 -1.4% [-2.1%, -0.7%]
WolfSheep small 🔵 -1.2% [-1.5%, -0.9%] 🔵 -1.4% [-1.5%, -1.3%]
WolfSheep large 🔵 -1.1% [-1.8%, -0.4%] 🔵 -2.1% [-2.6%, -1.6%]
BoidFlockers small 🔵 -1.2% [-1.8%, -0.7%] 🔵 -1.3% [-1.5%, -1.1%]
BoidFlockers large 🔵 -0.5% [-0.7%, -0.3%] 🔵 -0.3% [-0.6%, -0.1%]

@EwoutH
Copy link
Member

EwoutH commented Dec 1, 2025

While I do appreciate your effort, I thought I specifically requested one kind of warning at the time. That’s (also) to keep stuff reviewable for us. Both in size, as in scope (different reviewers have different expertises).

Could you open smaller, atomic PR’s that treats one kind of warning each? Especially split the visualization stuff from the RNG stuff.

Copy link
Member

Choose a reason for hiding this comment

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

Try not to include these kind of files

Copy link
Author

Choose a reason for hiding this comment

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

Ok @EwoutH I will

@ShreyasN707
Copy link
Author

@EwoutH Please review my PR.

@EwoutH
Copy link
Member

EwoutH commented Dec 2, 2025

Unfortunately you’re still doing multiple things at the same time.

Maybe it’s easier to split off some of the original commits to a new branch, and open a new PR?

@ShreyasN707
Copy link
Author

Unfortunately you’re still doing multiple things at the same time.

Maybe it’s easier to split off some of the original commits to a new branch, and open a new PR?

Okay sure i will fix the copy in my local machine and raise a new PR regarding the issue 2.1.

@ShreyasN707 ShreyasN707 closed this Dec 2, 2025
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