Remove exclude_none_values #1813
Conversation
|
With the latter, we may have multiple data collectors, but only 1 scheduler is needed. |
rht
force-pushed
the
remove_exclude_none_values
branch
from
2f30f78 to
4d0fb43
Compare
|
On a first impression it is a really good idea to use a filter function. With respect to this PR. I am in favor of removing the function, but technically this is a breaking change so we have to go to mesa 3.0. Being very handwavy we could say this is a bug fix, since the functions is broken atm. And since I doubt many people (if any) use this function we should get away with it. But we have to be aware that we are breaking semver. I am fine with that in this case, but let's see what are other people's opinions about it. |
This feature is functional only if the agent records are in the form of
```
{'satisfication': (('A0', 1), ('B0', None), ('A1', 1), ('B1', None), ('A2', 1), ('B2', None)), 'unique_id': (('A0', 'A0'), ('B0', 'B0'), ('A1', 'A1'), ('B1', 'B1'), ('A2', 'A2'), ('B2', 'B2'))}
```
instead of
```
((1, 'A0', 1, 'A0'), (1, 'B0', None, 'B0'), (1, 'A1', 1, 'A1'), (1, 'B1', None, 'B1'), (1, 'A2', 1, 'A2'), (1, 'B2', None, 'B2'))
```
A more explicit solution instead of implicitly ignoring a group of
agents just because of their attribute returns `None`, would be to add a
filter to the data collector itself. i.e., instead of
```python
agent_records = map(get_reports, model.schedule.agents)
```
we should do
```python
agent_records = map(get_reports, filter(agent_filter, model.schedule.agents))
```
Where `agent_filter` can be `lambda a: isinstance(a, Trader)`
. And this, too, is only a few lines of code of change.
rht
force-pushed
the
remove_exclude_none_values
branch
from
4d0fb43 to
ca11c4d
Compare
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## main #1813 +/- ##
==========================================
- Coverage 81.36% 81.34% -0.03%
==========================================
Files 15 15
Lines 891 879 -12
Branches 191 185 -6
==========================================
- Hits 725 715 -10
+ Misses 142 141 -1
+ Partials 24 23 -1
☔ View full report in Codecov by Sentry. |
|
This functionality hasn't been well advertised and without a tutorial/how-to guide, so in this case I think it is fine to consider this a bugfix. |
|
I am good with a bugfix as well; we want to get this in 2.1.2 or would this be 2.2? |
This feature is functional only if the agent records are in the form of
instead of
A more explicit solution instead of implicitly ignoring a group of agents just because of their attribute returns
None, would be to add a filter to the data collector itself. i.e., instead ofwe should do
Where
agent_filtercan belambda a: isinstance(a, Trader). And this, too, is only a few lines of code of change.