Bugfix in agentset to handle addition and removal correctly #1960
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1960 +/- ##
=======================================
Coverage 79.45% 79.45%
=======================================
Files 15 15
Lines 1285 1285
Branches 285 285
=======================================
Hits 1021 1021
Misses 225 225
Partials 39 39 ☔ View full report in Codecov by Sentry. |
mesa/agent.py
Outdated
| @@ -251,7 +251,8 @@ def do( | |||
| Returns: | |||
| AgentSet | list[Any]: The results of the method calls if return_results is True, otherwise the AgentSet itself. | |||
| """ | |||
| res = [getattr(agent, method_name)(*args, **kwargs) for agent in self._agents] | |||
| # we iterate over the actual weakref keys and check if weakref is alive before calling the method | |||
| res = [getattr(agent(), method_name)(*args, **kwargs) for agent in self._agents.keyrefs() if agent()] | |||
There was a problem hiding this comment.
I think it'd be clearer to name agent_ref instead of agent. Because the call agent_ref() is the actual agent themselves.
|
pre-commit.ci autofix |
for more information, see https://pre-commit.ci
|
It was a bit of a learning experience for me as well. I am familiar with weakrefs from Java so that helped. By the way, the code could have been even more elegant by using the walrus operator (first time I came across a reason to use it). res = [getattr(agent, method_name)(*args, **kwargs) for agentref in self._agents.keyrefs() if (agent := agentref())] |
That's true! I actually stumbled a bit while reading the code thinking about agentref being executed twice. But I didn't think of the walrus! But it's actually used in the space code for a similar list comprehension in get_cell_list_contents |
|
Should this result in a 2.2.1 release? |
|
In my opinion, yes. |
|
To inform you, we're currently blocked on our PyPI pipeline being broken (see #1962). I was supposed to meet with @jackiekazil to resolve that issue, but I unfortunately missed that meeting. Working on it. |
|
There was another "bug" we should fix first imho. |
To make sure I understand: @property
def agents_by_type(self):
agentsbytype = defauldict(dict)
for k, v in self._agents_by_type.items():
agentsbytype[k] = {agent:agent.unique_id for agent in v}
return agentsbytype |
|
Yes, but you might add a warning that the return type is deprecated and mesa 3.0 will return an Agentset (which imho makes much more sense) |
Agreed |
Fix for #1959 and additional unittests that test specifically on this