Repository: projectmesa/mesa

Issue:
bug: batch_run assumes model datacollector is called datacollector
**Describe the bug**
batch_run assumes user will call `DataCollector` `self.datacollector`. This is not annotated in the documentation, nor is it necessarily a requirement  

**Expected behavior**
users can name their model `DataCollector` whatever they want (e.g. `self.dc`)

**To Reproduce**
create model and name `DataCollector` anything else and then execute model with batch_run and try to retrieve the data. 

**Additional context**
Bug identified via students from the University of Mary Washington. They also had some very positive comments from their Professor
"They keep saying "wow, that's so easy! That makes it so fun!" Thanks to you and your crew for all you do."


Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/projectmesa/mesa/pull/1287?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=projectmesa) Report
> Merging [#1287](https://codecov.io/gh/projectmesa/mesa/pull/1287?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=projectmesa) (fe85f80) into [main](https://codecov.io/gh/projectmesa/mesa/commit/eb73aac0cf7bf129a4ac93187ff76a4fd9e3ae39?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=projectmesa) (eb73aac) will **decrease** coverage by `0.75%`.
> The diff coverage is `25.00%`.

```diff
@@            Coverage Diff             @@
##             main    #1287      +/-   ##
==========================================
- Coverage   89.30%   88.54%   -0.76%     
==========================================
  Files          19       19              
  Lines        1234     1240       +6     
  Branches      243      243              
==========================================
- Hits         1102     1098       -4     
- Misses         99      107       +8     
- Partials       33       35       +2     
```


| [Impacted Files](https://codecov.io/gh/projectmesa/mesa/pull/1287?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=projectmesa) | Coverage Δ | |
|---|---|---|
| [mesa/model.py](https://codecov.io/gh/projectmesa/mesa/pull/1287/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=projectmesa#diff-bWVzYS9tb2RlbC5weQ==) | `81.25% <25.00%> (-18.75%)` | :arrow_down: |
| [mesa/visualization/ModularVisualization.py](https://codecov.io/gh/projectmesa/mesa/pull/1287/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=projectmesa#diff-bWVzYS92aXN1YWxpemF0aW9uL01vZHVsYXJWaXN1YWxpemF0aW9uLnB5) | `74.04% <0.00%> (-1.96%)` | :arrow_down: |
| [mesa/visualization/UserParam.py](https://codecov.io/gh/projectmesa/mesa/pull/1287/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=projectmesa#diff-bWVzYS92aXN1YWxpemF0aW9uL1VzZXJQYXJhbS5weQ==) | `86.00% <0.00%> (-0.80%)` | :arrow_down: |
| [mesa/main.py](https://codecov.io/gh/projectmesa/mesa/pull/1287/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=projectmesa#diff-bWVzYS9tYWluLnB5) | `96.00% <0.00%> (-0.43%)` | :arrow_down: |
| [mesa/datacollection.py](https://codecov.io/gh/projectmesa/mesa/pull/1287/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=projectmesa#diff-bWVzYS9kYXRhY29sbGVjdGlvbi5weQ==) | `97.67% <0.00%> (-0.03%)` | :arrow_down: |
| [mesa/space.py](https://codecov.io/gh/projectmesa/mesa/pull/1287/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=projectmesa#diff-bWVzYS9zcGFjZS5weQ==) | `94.45% <0.00%> (-0.02%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/projectmesa/mesa/pull/1287?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=projectmesa).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=projectmesa)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/projectmesa/mesa/pull/1287?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=projectmesa). Last update [eb73aac...fe85f80](https://codecov.io/gh/projectmesa/mesa/pull/1287?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=projectmesa). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=projectmesa).

rht: By trying this PR, I did not mean for it to be merged. I meant for the PR to be installed as is.
jackiekazil: Should we add [WIP] in this case then?

On Thu, Apr 21, 2022 at 6:13 AM rht ***@***.***> wrote:

> By trying this PR, I did not mean for it to be merged. I meant for the PR
> to be installed as is.
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/projectmesa/mesa/pull/1287#issuecomment-1105010392>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AABIWTQAL6BDWG4LRQCB3KLVGES4NANCNFSM5T6FSEHA>
> .
> You are receiving this because you are subscribed to this thread.Message
> ID: ***@***.***>
>
-- 
Jacqueline Kazil | @jackiekazil

rht: @tpike3  this PR is ready.

Patch:
diff --git a/mesa/model.py b/mesa/model.py
index fddf68ea..6180a455 100644
--- a/mesa/model.py
+++ b/mesa/model.py
@@ -6,6 +6,8 @@ Core Objects: Model
 """
 import random
 
+from mesa.datacollection import DataCollector
+
 # mypy
 from typing import Any, Optional
 
@@ -61,3 +63,22 @@ class Model:
             seed = self._seed
         self.random.seed(seed)
         self._seed = seed
+
+    def initialize_data_collector(
+        self, model_reporters=None, agent_reporters=None, tables=None
+    ) -> None:
+        if not hasattr(self, "schedule") or self.schedule is None:
+            raise RuntimeError(
+                "You must initialize the scheduler (self.schedule) before initializing the data collector."
+            )
+        if self.schedule.get_agent_count() == 0:
+            raise RuntimeError(
+                "You must add agents to the scheduler before initializing the data collector."
+            )
+        self.datacollector = DataCollector(
+            model_reporters=model_reporters,
+            agent_reporters=agent_reporters,
+            tables=tables,
+        )
+        # Collect data for the first time during initialization.
+        self.datacollector.collect(self)


Test patch:
diff --git a/tests/test_datacollector.py b/tests/test_datacollector.py
index 0221b7da..0f2bd4c0 100644
--- a/tests/test_datacollector.py
+++ b/tests/test_datacollector.py
@@ -5,7 +5,6 @@ import unittest
 
 from mesa import Model, Agent
 from mesa.time import BaseScheduler
-from mesa.datacollection import DataCollector
 
 
 class MockAgent(Agent):
@@ -47,7 +46,7 @@ class MockModel(Model):
         for i in range(10):
             a = MockAgent(i, self, val=i)
             self.schedule.add(a)
-        self.datacollector = DataCollector(
+        self.initialize_data_collector(
             {
                 "total_agents": lambda m: m.schedule.get_agent_count(),
                 "model_value": "model_val",
@@ -103,10 +102,11 @@ class TestDataCollector(unittest.TestCase):
         assert "model_calc" in data_collector.model_vars
         assert "model_calc_comp" in data_collector.model_vars
         assert "model_calc_fail" in data_collector.model_vars
-        assert len(data_collector.model_vars["total_agents"]) == 7
-        assert len(data_collector.model_vars["model_value"]) == 7
-        assert len(data_collector.model_vars["model_calc"]) == 7
-        assert len(data_collector.model_vars["model_calc_comp"]) == 7
+        length = 8
+        assert len(data_collector.model_vars["total_agents"]) == length
+        assert len(data_collector.model_vars["model_value"]) == length
+        assert len(data_collector.model_vars["model_calc"]) == length
+        assert len(data_collector.model_vars["model_calc_comp"]) == length
         self.step_assertion(data_collector.model_vars["total_agents"])
         for element in data_collector.model_vars["model_value"]:
             assert element == 100
@@ -123,7 +123,7 @@ class TestDataCollector(unittest.TestCase):
         data_collector = self.model.datacollector
         agent_table = data_collector.get_agent_vars_dataframe()
 
-        assert len(data_collector._agent_records) == 7
+        assert len(data_collector._agent_records) == 8
         for step, records in data_collector._agent_records.items():
             if step < 5:
                 assert len(records) == 10
@@ -165,13 +165,35 @@ class TestDataCollector(unittest.TestCase):
         model_vars = data_collector.get_model_vars_dataframe()
         agent_vars = data_collector.get_agent_vars_dataframe()
         table_df = data_collector.get_table_dataframe("Final_Values")
-        assert model_vars.shape == (7, 5)
-        assert agent_vars.shape == (67, 2)
+        assert model_vars.shape == (8, 5)
+        assert agent_vars.shape == (77, 2)
         assert table_df.shape == (9, 2)
 
         with self.assertRaises(Exception):
             table_df = data_collector.get_table_dataframe("not a real table")
 
 
+class TestDataCollectorInitialization(unittest.TestCase):
+    def setUp(self):
+        self.model = Model()
+
+    def test_initialize_before_scheduler(self):
+        with self.assertRaises(RuntimeError) as cm:
+            self.model.initialize_data_collector()
+        self.assertEqual(
+            str(cm.exception),
+            "You must initialize the scheduler (self.schedule) before initializing the data collector.",
+        )
+
+    def test_initialize_before_agents_added_to_scheduler(self):
+        with self.assertRaises(RuntimeError) as cm:
+            self.model.schedule = BaseScheduler(self)
+            self.model.initialize_data_collector()
+        self.assertEqual(
+            str(cm.exception),
+            "You must add agents to the scheduler before initializing the data collector.",
+        )
+
+
 if __name__ == "__main__":
     unittest.main()