Rename 'agents' property to 'df' for AgentContainer for issue # 68#148
Conversation
chnages done in AgentContainer to move agents to df and change done as rquied in agents/agentset in abstract/concrete. Note: i have depreciated agents to use df instead to keep it compatible with others and will remove once all looks good
WalkthroughThe changes systematically rename the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AgentSetDF
participant DataFrame
User->>AgentSetDF: Access .df property
AgentSetDF->>DataFrame: Return underlying DataFrame
User->>AgentSetDF: Set .df property
AgentSetDF->>DataFrame: Update underlying DataFrame
User->>AgentSetDF: Access .agents property (deprecated)
AgentSetDF->>User: Emit DeprecationWarning
AgentSetDF->>DataFrame: Return underlying DataFrame
Poem
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
for more information, see https://pre-commit.ci
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #148 +/- ##
=======================================
Coverage 89.88% 89.88%
=======================================
Files 12 12
Lines 1780 1780
=======================================
Hits 1600 1600
Misses 180 180 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
adamamer20
left a comment
adamamer20
left a comment
There was a problem hiding this comment.
Seems a good direction!
However we are still very early in the development phase so we can deprecate without giving notice to users.
Try removing completely the agents property and test if everything (including examples) work correctly
removing agents property completely, instead use df property, also replacing agents with df in tests and boltzmann_wealth example
for more information, see https://pre-commit.ci
removed agents property completely to use df, all tests passed. Boltzmann_wealth is updated and passed. no change found in sugarscape example |
|
Alright, before I proceed with my final review, I’d like to tag @Ben-geo to get his thoughts as well. I think this would be a good opportunity for him to test out the review process and share his feedback. |
|
Thanks a lot for the tag, @adamamer20 ! I went through the implementation and didn’t find anything that needs to be changed. Everything looks solid. (Though there's always a chance I might have missed something, first review 😅) and I was thinking about how will it work after including #146 I like this better but it still does feel like a lot for the end user right? @reyan-singh what do you think?? |
Thanks @Ben-geo for review, yes need to use df instead of agents for agentsets, one of the reason is this give clear hint to users its data frame but more then that as described in issue also this new syntax df avoid confusion when used with model and nested agents |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Pull Request Overview
This PR renames the agents property to df in AgentContainer implementations, deprecates the old alias, and updates all tests and examples to use the new API.
- Renamed property and setter from
agentstodfin both concrete and abstract classes. - Added deprecation warnings for the old
agentsproperty in the abstract base. - Updated tests and example scripts to reference
dfinstead ofagents.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/test_agentset.py | Updated assertions to use df instead of agents. |
| tests/test_agents.py | Refactored tests for AgentsDF to reference the df property. |
| mesa_frames/concrete/agentset.py | Renamed agents property to df and imported warnings. |
| mesa_frames/concrete/agents.py | Renamed container property to df. |
| mesa_frames/abstract/agents.py | Introduced df property/setter and deprecation warnings for agents. |
| examples/boltzmann_wealth/performance_plot.py | Adapted example code to use df for sampling and updating frames. |
Comments suppressed due to low confidence (2)
mesa_frames/concrete/agents.py:558
- The
dfproperty returns each agentset's oldagentsproperty rather than its newdf. It should returnagentset.dfto align with the rename.
return {agentset: agentset.agents for agentset in self._agentsets}
mesa_frames/concrete/agentset.py:504
- [nitpick] The setter for
dfusesagentsas its parameter name, which can be confusing. Rename the argument todfordata_framefor clarity.
def df(self, agents: pl.DataFrame) -> None:
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
mesa_frames/concrete/agents.py (1)
65-65: Remove unused importThe
warningsmodule is imported but not used in this file. It should be removed to keep the imports clean.-import warnings🧰 Tools
🪛 Ruff (0.11.9)
65-65:
warningsimported but unusedRemove unused import:
warnings(F401)
mesa_frames/concrete/agentset.py (1)
76-76: Remove unused importThe
warningsmodule is imported but not used in this file. It should be removed to keep the imports clean.-import warnings🧰 Tools
🪛 Ruff (0.11.9)
76-76:
warningsimported but unusedRemove unused import:
warnings(F401)
tests/test_agentset.py (1)
276-276: Replaceassert Falsewithraise AssertionError()The
assert Falsestatement should be replaced withraise AssertionError()becauseassertstatements are removed when Python is run with the-Ooptimization flag.- assert False + raise AssertionError("Failed to shuffle agents after 10 attempts")🧰 Tools
🪛 Ruff (0.11.9)
276-276: Do not
assert False(python -Oremoves these calls), raiseAssertionError()Replace
assert False(B011)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
examples/boltzmann_wealth/performance_plot.py(3 hunks)mesa_frames/abstract/agents.py(4 hunks)mesa_frames/concrete/agents.py(2 hunks)mesa_frames/concrete/agentset.py(2 hunks)tests/test_agents.py(18 hunks)tests/test_agentset.py(12 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (5)
mesa_frames/concrete/agents.py (4)
mesa_frames/concrete/agentset.py (2)
df(500-501)df(504-507)mesa_frames/abstract/agents.py (7)
df(652-658)df(662-668)df(1044-1045)df(1048-1056)AgentSetDF(728-1101)agents(1059-1065)agents(1068-1081)mesa_frames/concrete/model.py (2)
agents(162-180)agents(183-186)mesa_frames/abstract/space.py (1)
agents(510-517)
mesa_frames/concrete/agentset.py (4)
mesa_frames/concrete/agents.py (2)
df(557-558)df(561-569)mesa_frames/abstract/agents.py (6)
df(652-658)df(662-668)df(1044-1045)df(1048-1056)agents(1059-1065)agents(1068-1081)mesa_frames/concrete/model.py (2)
agents(162-180)agents(183-186)mesa_frames/abstract/space.py (1)
agents(510-517)
examples/boltzmann_wealth/performance_plot.py (2)
mesa_frames/concrete/agentset.py (4)
df(500-501)df(504-507)active_agents(510-511)active_agents(514-515)mesa_frames/abstract/agents.py (7)
df(652-658)df(662-668)df(1044-1045)df(1048-1056)active_agents(672-678)active_agents(682-693)active_agents(1085-1085)
tests/test_agentset.py (3)
mesa_frames/abstract/agents.py (32)
agents(1059-1065)agents(1068-1081)df(652-658)df(662-668)df(1044-1045)df(1048-1056)add(106-125)add(747-771)pos(718-725)pos(1095-1101)discard(83-103)discard(773-788)active_agents(672-678)active_agents(682-693)active_agents(1085-1085)do(152-160)do(164-172)do(175-206)do(791-799)do(802-810)do(812-846)select(263-291)get(210-210)get(214-216)get(219-238)get(850-854)get(858-862)get(865-869)remove(241-260)remove(876-885)shuffle(345-357)sort(360-385)mesa_frames/concrete/model.py (2)
agents(162-180)agents(183-186)mesa_frames/concrete/agentset.py (10)
df(500-501)df(504-507)add(102-151)pos(526-527)active_agents(510-511)active_agents(514-515)select(237-256)get(170-182)shuffle(258-265)sort(267-280)
mesa_frames/abstract/agents.py (4)
mesa_frames/concrete/agents.py (2)
df(557-558)df(561-569)mesa_frames/concrete/agentset.py (2)
df(500-501)df(504-507)mesa_frames/concrete/model.py (2)
agents(162-180)agents(183-186)mesa_frames/abstract/space.py (1)
agents(510-517)
🪛 Ruff (0.11.9)
mesa_frames/concrete/agents.py
65-65: warnings imported but unused
Remove unused import: warnings
(F401)
mesa_frames/concrete/agentset.py
76-76: warnings imported but unused
Remove unused import: warnings
(F401)
tests/test_agentset.py
276-276: Do not assert False (python -O removes these calls), raise AssertionError()
Replace assert False
(B011)
tests/test_agents.py
458-458: Do not assert False (python -O removes these calls), raise AssertionError()
Replace assert False
(B011)
🪛 GitHub Check: codecov/patch
mesa_frames/abstract/agents.py
[warning] 1045-1045: mesa_frames/abstract/agents.py#L1045
Added line #L1045 was not covered by tests
[warning] 1056-1056: mesa_frames/abstract/agents.py#L1056
Added line #L1056 was not covered by tests
[warning] 1076-1076: mesa_frames/abstract/agents.py#L1076
Added line #L1076 was not covered by tests
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build (windows, 3.12)
🔇 Additional comments (70)
mesa_frames/concrete/agents.py (2)
557-558: LGTM! Property renamed for clarityThe property rename from
agentstodfclearly indicates that this returns a DataFrame, which aligns with the PR objectives.
560-569: LGTM! Setter properly renamedThe setter for the property has been correctly renamed from
agentstodfwhile maintaining the same functionality.mesa_frames/concrete/agentset.py (2)
500-501: LGTM! Property renamed for clarityThe property rename from
agentstodfclearly indicates that this returns a DataFrame, which aligns with the PR objectives.
503-507: LGTM! Setter properly renamedThe setter for the property has been correctly renamed from
agentstodfwhile maintaining the same functionality and validation.examples/boltzmann_wealth/performance_plot.py (4)
104-104: LGTM! Property access updatedThe code has been correctly updated to use
self.dfinstead ofself.agentsfor sampling agents.
141-141: LGTM! Property access updatedThe code has been correctly updated to use
self.dfinstead ofself.agentsfor sampling agents.
144-148: LGTM! Property access updatedThe code has been correctly updated to use
self.dfinstead ofself.agentsfor updating agent wealth.
153-158: LGTM! Property access updatedThe code has been correctly updated to use
self.dfinstead ofself.agentsfor joining with the new wealth data.tests/test_agentset.py (39)
78-79: LGTM! Test updated to use new property nameTest has been properly updated to use
agents.dfinstead ofagents.agents, aligning with the property rename.
93-95: LGTM! Test updated to use new property nameThe test for adding agents with a DataFrame has been properly updated to use the new
dfproperty.
97-100: LGTM! Test updated to use new property nameThe test for adding agents with a sequence has been properly updated to use the new
dfproperty.
104-106: LGTM! Test updated to use new property nameThe test for adding agents with a dictionary has been properly updated to use the new
dfproperty.
137-138: LGTM! Test updated to use new property nameThe test for discarding agents has been properly updated to use the new
dfproperty.
145-146: LGTM! Test updated to use new property nameThe test for discarding agents with a list has been properly updated to use the new
dfproperty.
152-153: LGTM! Test updated to use new property nameThe test for discarding agents with a DataFrame has been properly updated to use the new
dfproperty.
160-161: LGTM! Test updated to use new property nameThe test for discarding active agents has been properly updated to use the new
dfproperty.
167-168: LGTM! Test updated to use new property nameThe test for discarding with an empty list has been properly updated to use the new
dfproperty.
174-174: LGTM! Test updated to use new property nameThe test for the
domethod with no return_results and no mask has been properly updated to use the newdfproperty.
178-178: LGTM! Test updated to use new property nameThe test for the
domethod with return_results=True and no mask has been properly updated to use the newdfproperty.
182-182: LGTM! Test updated to use new property nameThe test for the
domethod with a mask has been properly updated to use the newdfproperty.
194-194: LGTM! Test updated to use new property nameThe test for getting attributes has been properly updated to use the new
dfproperty.
197-197: LGTM! Test updated to use new property nameThe test for selecting agents has been properly updated to use the new
dfproperty.
203-203: LGTM! Test updated to use new property nameThe test for removing agents has been properly updated to use the new
dfproperty.
213-213: LGTM! Test updated to use new property nameThe test for selecting all agents has been properly updated to use the new
dfproperty.
233-233: LGTM! Test updated to use new property nameThe filter function in the selection test has been properly updated to use the new
dfproperty.
252-252: LGTM! Test updated to use new property nameThe test for setting a single attribute has been properly updated to use the new
dfproperty.
256-257: LGTM! Test updated to use new property nameThe test for setting multiple attributes has been properly updated to use the new
dfproperty.
260-260: LGTM! Test updated to use new property nameThe test for selecting with a condition has been properly updated to use the new
dfproperty.
262-262: LGTM! Test updated to use new property nameThe test for setting attributes with a mask has been properly updated to use the new
dfproperty.
266-267: LGTM! Test updated to use new property nameThe test for setting attributes with a dictionary has been properly updated to use the new
dfproperty.
272-274: LGTM! Test updated to use new property nameThe test for shuffling agents has been properly updated to use the new
dfproperty.
281-281: LGTM! Test updated to use new property nameThe test for sorting agents has been properly updated to use the new
dfproperty.
292-293: LGTM! Test updated to use new property nameThe test for adding agents with the
__add__operator has been properly updated to use the newdfproperty.
297-300: LGTM! Test updated to use new property nameThe test for adding agents with a sequence has been properly updated to use the new
dfproperty.
304-305: LGTM! Test updated to use new property nameThe test for adding agents with a dictionary has been properly updated to use the new
dfproperty.
362-363: LGTM! Test updated to use new property nameThe test for the
__iadd__operator with a DataFrame has been properly updated to use the newdfproperty.
368-371: LGTM! Test updated to use new property nameThe test for the
__iadd__operator with a list has been properly updated to use the newdfproperty.
376-377: LGTM! Test updated to use new property nameThe test for the
__iadd__operator with a dictionary has been properly updated to use the newdfproperty.
388-389: LGTM! Test updated to use new property nameThe test for the
__isub__operator has been properly updated to use the newdfproperty.
413-414: LGTM! Test updated to use new property nameThe test for setting items with a string key has been properly updated to use the new
dfproperty.
417-418: LGTM! Test updated to use new property nameThe test for setting items with a list of strings has been properly updated to use the new
dfproperty.
422-422: LGTM! Test updated to use new property nameThe test for setting items with a tuple key has been properly updated to use the new
dfproperty.
426-427: LGTM! Test updated to use new property nameThe test for setting items with an agent mask has been properly updated to use the new
dfproperty.
435-437: LGTM! Test updated to use new property nameThe test for the
__sub__operator has been properly updated to use the newdfproperty.
451-455: LGTM! Test updated to use new property nameThe test for the
agentsproperty (nowdf) has been properly updated to use the new property name.
461-465: LGTM! Test updated to use new property nameThe test for
active_agentshas been properly updated to use the newdfproperty.
471-471: LGTM! Test updated to use new property nameThe test for
inactive_agentshas been properly updated to use the newdfproperty.tests/test_agents.py (18)
149-152: Update reference todfproperty successfully.The reference to
result._agentsets[1].df["unique_id"]correctly uses the newdfproperty instead of the deprecatedagentsproperty.
164-168: Consistent property renaming applied.All references to the
agentsproperty have been replaced withdf, maintaining consistency across the codebase.
197-202: Mask creation using the newdfproperty.The masks are now correctly created using the
dfproperty instead ofagents.
303-304: Consistent property renaming in test_remove method.The reference to
result._agentsets[1].df["unique_id"]correctly uses the new property name.
364-364: Filter function updated to usedfproperty.The filter function now correctly uses
agentset.df["wealth"]instead of accessing the deprecatedagentsproperty.
450-455: Testing shuffle functionality with updated property references.The references to the
dfproperty in the shuffle test correctly reflect the property renaming.
463-464: Sort testing with updated property references.The sort test correctly uses the
dfproperty to verify that agents are sorted by wealth in descending order.
479-484: Step test updated to usedfproperty.The step test correctly verifies the updated wealth values using the
dfproperty.
542-544: Mask creation in test__get_bool_masks updated.The mask creation in the
test__get_bool_masksmethod now correctly uses thedfproperty.
554-554: Updated reference in testing bool masks with IdsLike.The test now correctly references
agents._agentsets[1].df["unique_id"][0]instead of using the deprecatedagentsproperty.
669-673: Test__getattr__ method updated.The test for
__getattr__now correctly verifies that the returned values match those from thedfproperty.
731-735: Updated test__getitem__ with mask.The test for
__getitem__with a mask now correctly references thedfproperty in the assertions.
778-782: Test__iter__ method updated.The test for
__iter__now correctly verifies that the agent's unique ID matches the one from thedfproperty.
873-876: Test for the newdfproperty.The test for the
agentsproperty has been updated to test the newdfproperty instead, while keeping the same verification logic.
879-879: Setter fordfproperty tested.The test now correctly calls the
dfsetter instead of the deprecatedagentssetter.
887-888: Active_agents test updated to usedfproperty.The test for the
active_agentsproperty now correctly creates masks using thedfproperty.
943-944: Agentsets_by_type test updated.The test for the
agentsets_by_typeproperty now correctly uses thedfproperty to access agent data.
951-952: Inactive_agents test updated to usedfproperty.The test for the
inactive_agentsproperty now correctly creates masks using thedfproperty.mesa_frames/abstract/agents.py (5)
64-64: Added warnings module for deprecation notices.The import of the
warningsmodule is added to support the deprecation warnings for theagentsproperty.
652-652: Renamed abstract property fromagentstodf.The abstract property in the
AgentContainerclass has been renamed fromagentstodf, which better indicates that it represents a data frame.
660-662: Renamed abstract property setter fromagentstodf.The abstract property setter has been renamed from
agentstodfto match the property name change, maintaining consistency in the API.
1058-1066: Added deprecation warning toagentsproperty.The
agentsproperty now emits a deprecation warning to guide users toward using the newdfproperty instead. This is a good practice for maintaining backward compatibility while encouraging migration to the new API.
1076-1081: Added deprecation warning toagentssetter.The
agentssetter now emits a deprecation warning similar to the property getter, consistently guiding users to the newdfsetter.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 1076-1076: mesa_frames/abstract/agents.py#L1076
Added line #L1076 was not covered by tests
…o pr/reyan-singh/148
for more information, see https://pre-commit.ci
…DF, AgentsDF, and AgentSetPolars classes
for more information, see https://pre-commit.ci
…a#68' of https://github.com/reyan-singh/mesa-frames into pr/reyan-singh/148
adamamer20
added
breaking
Changes done in AgentContainer to rename agents as df and change done as required in agents/agentset classes to reflect df in abstract/concrete files
Note: i have depreciated agents to use df instead, to keep it compatible with existing files if any and then good to remove completely once all looks good
Summary by CodeRabbit
agentstodfacross the codebase for improved consistency.agentsproperty in favor ofdf, with warnings provided for legacy usage.dfattribute instead ofagentsto ensure compatibility with the refactored code.