Make RandomActivationByType.agents_by_type backward compatible

[quaquel]

As #pointed out by @Corvince, RandomActivationByType.agents_by_type in 2.2 behaved differently from 2.1.5. This is a simple fix for this, with a warning added to raise awareness of future changes.

[EwoutH]

Thanks for this. Can we add an optional keyword argument named something like return_agentset, that's now False by default and emits the deprecation warning? Because then users can already set it to True to get the new behavior.

Or should users migrate to model.agents? But then if we want to keep schedulers it can mean that the scheduler only has a subset of agents, and thus isn't equivalent to model.agents.

(in general, when deprecating something a replacement should be available)

[quaquel]

Thanks for this. Can we add an optional keyword argument named something like return_agentset, that's now False by default and emits the deprecation warning? Because then users can already set it to True to get the new behavior.

Or should users migrate to model.agents? But then if we want to keep schedulers it can mean that the scheduler only has a subset of agents, and thus isn't equivalent to model.agents.

(in general, when deprecating something a replacement should be available)

It's a property so it behaves like an attribute from the outside (as in 2.1.5). The actual data is available via _agents_by_type. We might include this information in the warning. In the future (i.e. 3.0), the return type of agents_by_type will change and this descriptor can be removed (and the attribute name updated by removing the leading underscore).

[EwoutH]

Adding that information to the warning sounds good!

[Corvince]

Thanks a lot @quaquel ! I approved the PR, but noticed that we have the same Problem for schedule.agents, which now also returns an AgentSet instead of list[Agent]. I am afraid we have to change this one as well. You can decide if you want to make it part of this PR or a new one

[quaquel]

I'll fix it here as soon as possible (have some meetings coming up)

[codecov]

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

Comparison is base (9819927) 79.45% compared to head (1f9af49) 79.19%.
Report is 1 commits behind head on main.

Files	Patch %	Lines
mesa/time.py	64.70%	6 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1965      +/-   ##
==========================================
- Coverage   79.45%   79.19%   -0.26%     
==========================================
  Files          15       15              
  Lines        1285     1293       +8     
  Branches      285      260      -25     
==========================================
+ Hits         1021     1024       +3     
- Misses        225      230       +5     
  Partials       39       39              

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

[quaquel]

Thanks a lot @quaquel ! I approved the PR, but noticed that we have the same Problem for schedule.agents, which now also returns an AgentSet instead of list[Agent]. I am afraid we have to change this one as well. You can decide if you want to make it part of this PR or a new one

Actually, I'm not sure this needs fixing at all. We discussed it earlier in the original PR. AgentSet is a sequence, so it behaves like a list.

[Corvince]

Ah, you are correct! Didn't think of it that way. But I agree, the use cases where the difference matters are hopefully not used in the wild, so I think we can close an eye on semver in this case. Btw is it possible to call list(AgentSet) to turn it into a list?

[quaquel]

What dunder method would be called when you do that?

update: I just checked and it works. list(AgentSet) simply gives you a list of agents as expected.

[quaquel]

So, let's get this merged and get 2.2.1 online next.