Skip to content

Conversation

@madhavik-2005
Copy link

Summary

Fixes #2896

This PR fixes the GroupBy.__iter__ method that was causing test_agentset_groupby to fail with AttributeError: 'GroupBy' object has no attribute '_agents'.

Problem

The GroupBy.__iter__ method was incorrectly trying to access self._agents.keyrefs(), but GroupBy doesn't have an _agents attribute. The GroupBy class stores its data in self.groups (a dictionary), not _agents.

Solution

Changed GroupBy.__iter__ to correctly iterate over the groups dictionary:

Before:

def __iter__(self):
    for ref in self._agents.keyrefs():
        if (agent := ref()) is not None:
            yield agent

After:

def __iter__(self) -> Iterator[tuple[Hashable, list | AgentSet]]:
    """Iterate over (group_name, group) tuples."""
    return iter(self.groups.items())

This now correctly returns (group_name, group) tuples as expected by the test suite and matches how GroupBy is intended to be used.

Testing

Added tests/test_iteration_fix.py with a test that forces garbage collection during iteration to ensure robustness:

def test_agentset_iteration_with_gc():
    """Test that AgentSet iteration doesn't fail when GC runs during iteration."""
    model = Model()
    agents = [TestAgent(model) for _ in range(100)]
    agentset = AgentSet(agents, random=model.random)
    
    collected_ids = []
    for i, agent in enumerate(agentset):
        collected_ids.append(agent.unique_id)
        if i % 10 == 0:
            gc.collect()
    
    assert len(collected_ids) == 100

Test Results:

  • ✅ New GC iteration test passes
  • ✅ All 22 tests in test_agent.py pass (including the previously failing test_agentset_groupby)

Additional Context

This fix ensures that iterating over GroupBy objects works as intended and prevents the AttributeError that was blocking the test suite.

madhavik-2005 and others added 2 commits November 27, 2025 17:38
- Fix GroupBy.__iter__ to iterate over groups.items() instead of non-existent _agents
@github-actions
Copy link

Performance benchmarks:

Model Size Init time [95% CI] Run time [95% CI]
BoltzmannWealth small 🔵 +1.6% [+0.8%, +2.6%] 🔵 -1.2% [-1.6%, -0.9%]
BoltzmannWealth large 🔵 -1.2% [-2.0%, -0.5%] 🔵 -2.4% [-3.9%, -1.2%]
Schelling small 🔵 -0.9% [-1.2%, -0.6%] 🔵 -0.8% [-1.0%, -0.7%]
Schelling large 🔵 -0.9% [-1.3%, -0.4%] 🔵 -3.3% [-4.3%, -2.4%]
WolfSheep small 🔵 -2.7% [-3.0%, -2.4%] 🔵 -1.6% [-2.2%, -1.1%]
WolfSheep large 🔵 -3.4% [-4.4%, -2.4%] 🔵 -3.7% [-5.8%, -1.6%]
BoidFlockers small 🔵 -1.1% [-1.8%, -0.5%] 🔵 +1.9% [+1.6%, +2.1%]
BoidFlockers large 🔵 -1.4% [-2.1%, -0.7%] 🔵 +0.0% [-0.9%, +1.0%]

@tpike3
Copy link
Member

tpike3 commented Nov 27, 2025

@madhavik-2005 Thanks!

  • Could you please add the docstring you put n the PR """Iterate over (group_name, group) tuples.""" into the code. That should stop the pre-commit error.
  • The readthedocs failure looks like its an issue with solara since it is failing on import of mesa.visualization. That one will take a little more digging.

@tpike3 tpike3 requested a review from EwoutH November 27, 2025 13:56
@madhavik-2005
Copy link
Author

Hi @tpike3,

I've fixed the docstring issue. I had accidentally removed part of the docstring that pre-commit had added. The full docstring is now back in place:

"""Iterate over (group_name, group) tuples.

Yields:
tuple[Hashable, list | AgentSet]: pairs of group name and group.
"""

The pre-commit checks should pass now. Please let me know if there are any other changes needed - I'm still learning the contribution workflow and appreciate your guidance!

@EwoutH
Copy link
Member

EwoutH commented Nov 27, 2025

I think I made a mistake, and filled this bug wrongly. I think it's actually not at our end, but Solara's: widgetti/solara#1107. See also #2843.

Sorry, appreciate the effort.

@madhavik-2005
Copy link
Author

Thanks for the update! Since this issue is actually on Solara’s side, I’ll close this PR and follow the Solara issue (widgetti/solara#1107). Appreciate the guidance!

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.

AgentSet iteration fails on Python 3.14 with "dictionary changed size during iteration"

3 participants