-
Notifications
You must be signed in to change notification settings - Fork 14
Generate automatically unique_id #110
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…sDF for handling the unique_id
@adamamer20 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added exception for adding unique id
Since we handle ids internally, I think the burden of keeping them unique is on us. |
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
@adamamer20 Right now polars backend passes all tests, the pandas one still needs some fixes, mainly adapting the current unit tests for using automatically generated unique ids. Things that need confirmation
|
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think the code is well written. delete the changes for pandas, and make that change in id generation.
for more information, see https://pre-commit.ci
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis update enhances agent ID handling across the codebase. The changes switch several internal agent ID types to unsigned 64-bit integers (uint64) and adjust method signatures and DataFrame constructions accordingly. New unique ID generation methods have been added for both Pandas and Polars implementations. Minor adjustments in mixin classes (such as docstring formatting) and changes in tests to dynamically reference agent IDs further streamline the functionality. Overall, these modifications standardize and improve the robustness of agent data management. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant A as AgentSet (Pandas/Polars)
participant U as _generate_unique_ids
participant DF as DataFrame
C->>A: Call add(agents)
alt Missing unique IDs
A->>U: Generate unique IDs
U-->>A: Return new IDs
end
A->>DF: Update agents with uint64 IDs
DF-->>A: Updated DataFrame
A-->>C: Return updated AgentSet
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
tests/pandas/test_grid_pandas.py (1)
700-700
: 💡 Verification agent🧩 Analysis chain
Fix agent ID validation in test methods.
The pipeline failures indicate "ValueError: Some agents are not in the model" in several test methods. Ensure that agent IDs used in these methods are correctly retrieved using
get_unique_ids()
and that you're using valid indices.Also applies to: 744-744, 888-888, 932-932
🏁 Script executed:
#!/bin/bash # Find all occurrences of methods that might be causing the errors rg -A 2 "test_move_to_available|test_move_to_empty|test_place_to_available|test_place_to_empty" tests/pandas/test_grid_pandas.pyLength of output: 601
Fix agent ID validation in test methods
The pipeline failures (“ValueError: Some agents are not in the model”) suggest that the tests are using hard-coded or invalid agent IDs (e.g. a literal “0”) rather than dynamically retrieving them. Please update the following test methods in
tests/pandas/test_grid_pandas.py
so that they obtain valid agent IDs viagrid_moore.get_unique_ids()
before using them:
test_move_to_available
(line 700)test_move_to_empty
(line 744)test_place_to_available
(line 888)test_place_to_empty
(line 932)Once revised, re-run the test suite to ensure that the “Some agents are not in the model” error no longer occurs.
🧰 Tools
🪛 GitHub Actions: build
[error] 700-700: ValueError: Some agents are not in the model
🧹 Nitpick comments (11)
tests/pandas/test_grid_pandas.py (1)
8-8
: Unused import "AgentSetDF".The import
mesa_frames.abstract.agents.AgentSetDF
is not used in the file.-from mesa_frames.abstract.agents import AgentSetDF
🧰 Tools
🪛 Ruff (0.8.2)
8-8:
mesa_frames.abstract.agents.AgentSetDF
imported but unusedRemove unused import:
mesa_frames.abstract.agents.AgentSetDF
(F401)
tests/polars/test_agentset_polars.py (1)
100-107
: Verify comparison method in test assertions.When comparing Series objects, use of
all()
may not guarantee proper element-wise comparison. Consider using more robust comparison methods likepl.Series.equals()
or converting to lists before comparison.-assert all( - result["unique_id"] - == pl.concat([agents["unique_id"], agents2["unique_id"]]) -) +assert result["unique_id"].equals( + pl.concat([agents["unique_id"], agents2["unique_id"]]) +)Also applies to: 316-321
mesa_frames/abstract/space.py (2)
57-57
: Remove unused importpandas
The
pandas
module is imported but not used in this file. This is confirmed by the static analyzer which flagged it as an unused import (F401).-import pandas as pd
🧰 Tools
🪛 Ruff (0.8.2)
57-57:
pandas
imported but unusedRemove unused import:
pandas
(F401)
66-66
: Remove unused importAgnosticIds
The
AgnosticIds
type is imported frommesa_frames.types_
but not used in this file. Static analysis confirms this as an unused import (F401).- AgnosticIds,
🧰 Tools
🪛 Ruff (0.8.2)
66-66:
mesa_frames.types_.AgnosticIds
imported but unusedRemove unused import:
mesa_frames.types_.AgnosticIds
(F401)
tests/test_agents.py (1)
214-218
: Add a comment explaining the reindexingThe code now saves the original index order and reindexes after the operation. This is good practice to ensure test stability, but it would be helpful to add a comment explaining why this is necessary.
original_index_for_pandas_df = agents._agentsets[0].index.copy() agents.do("add_wealth", 1, mask=mask_dictionary) +# Reindex to ensure consistent ordering for test assertions, as do() method may change order agents._agentsets[0].agents = agents._agentsets[0].agents.reindex( original_index_for_pandas_df )
mesa_frames/concrete/pandas/agentset.py (1)
121-121
: Consider rewriting Yoda condition.
The lineif "unique_id" == agents.index.name or ...
might read more naturally asif agents.index.name == "unique_id"
.- if "unique_id" == agents.index.name or "unique_id" in agents.columns: + if agents.index.name == "unique_id" or "unique_id" in agents.columns:🧰 Tools
🪛 Ruff (0.8.2)
121-121: Yoda condition detected
Rewrite as
agents.index.name == "unique_id"
(SIM300)
mesa_frames/concrete/polars/agentset.py (2)
62-63
: Remove unused imports.
Theuuid
andwarnings
imports are not referenced in this file.- import uuid - import warnings🧰 Tools
🪛 Ruff (0.8.2)
62-62:
uuid
imported but unusedRemove unused import:
uuid
(F401)
63-63:
warnings
imported but unusedRemove unused import:
warnings
(F401)
220-220
: Clarify exception chaining.
Suggest usingraise KeyError(error) from error
to preserve the original traceback.- raise KeyError(error) + raise KeyError(error) from error🧰 Tools
🪛 Ruff (0.8.2)
220-220: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
tests/polars/test_grid_polars.py (3)
34-35
: Commented code should be removedThe commented-out sorting operations are no longer needed with the automatic unique ID generation system, but they should be removed rather than left as comments.
- # fix1_AgentSetPandas.agents = fix1_AgentSetPandas.agents.sort_index(inplace=False) - # fix2_AgentSetPolars.agents = fix2_AgentSetPolars.agents.sort("unique_id")
975-978
: Simplify nested conditionalThis nested if statement can be simplified.
- if last is not None and not different: - if ( - space.agents.select(pl.col("dim_0", "dim_1")).to_numpy() != last - ).any(): - different = True + if last is not None and not different and ( + space.agents.select(pl.col("dim_0", "dim_1")).to_numpy() != last + ).any(): + different = True🧰 Tools
🪛 Ruff (0.8.2)
975-978: Use a single
if
statement instead of nestedif
statementsCombine
if
statements usingand
(SIM102)
1312-1315
: Simplify nested conditionalSimilar to the earlier case, this nested if statement can be simplified.
- if last is not None and not different: - if ( - space.agents.select(pl.col("dim_0", "dim_1")).to_numpy() != last - ).any(): - different = True + if last is not None and not different and ( + space.agents.select(pl.col("dim_0", "dim_1")).to_numpy() != last + ).any(): + different = True🧰 Tools
🪛 Ruff (0.8.2)
1312-1315: Use a single
if
statement instead of nestedif
statementsCombine
if
statements usingand
(SIM102)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
mesa_frames/abstract/space.py
(3 hunks)mesa_frames/concrete/agents.py
(7 hunks)mesa_frames/concrete/pandas/agentset.py
(9 hunks)mesa_frames/concrete/pandas/mixin.py
(1 hunks)mesa_frames/concrete/polars/agentset.py
(9 hunks)mesa_frames/concrete/polars/mixin.py
(2 hunks)tests/pandas/test_agentset_pandas.py
(15 hunks)tests/pandas/test_grid_pandas.py
(27 hunks)tests/polars/test_agentset_polars.py
(13 hunks)tests/polars/test_grid_polars.py
(32 hunks)tests/test_agents.py
(12 hunks)
🧰 Additional context used
🧬 Code Definitions (7)
tests/pandas/test_grid_pandas.py (2)
mesa_frames/concrete/pandas/space.py (1)
remaining_capacity
(235-238)mesa_frames/concrete/pandas/agentset.py (4)
agents
(434-435)agents
(438-445)pos
(464-465)index
(460-461)
tests/polars/test_agentset_polars.py (1)
mesa_frames/concrete/polars/agentset.py (11)
agents
(541-542)agents
(545-548)add
(105-161)contains
(164-164)contains
(167-167)contains
(169-178)active_agents
(551-552)active_agents
(555-556)select
(253-272)AgentSetPolars
(83-568)inactive_agents
(559-560)
tests/test_agents.py (1)
mesa_frames/concrete/pandas/agentset.py (6)
agents
(434-435)agents
(438-445)contains
(155-155)contains
(158-158)contains
(160-170)index
(460-461)
tests/pandas/test_agentset_pandas.py (1)
mesa_frames/concrete/pandas/agentset.py (13)
agents
(434-435)agents
(438-445)pos
(464-465)index
(460-461)add
(112-152)contains
(155-155)contains
(158-158)contains
(160-170)active_agents
(448-449)active_agents
(452-453)select
(230-250)AgentSetPandas
(76-465)inactive_agents
(456-457)
mesa_frames/concrete/pandas/agentset.py (4)
mesa_frames/concrete/polars/agentset.py (4)
add
(105-161)agents
(541-542)agents
(545-548)_generate_unique_ids
(485-493)mesa_frames/concrete/agents.py (3)
add
(88-117)agents
(553-554)agents
(557-565)mesa_frames/abstract/agents.py (8)
add
(104-123)add
(746-770)agents
(651-657)agents
(661-667)agents
(1043-1044)agents
(1047-1055)AgentSetDF
(727-1075)random
(630-637)mesa_frames/concrete/model.py (2)
agents
(162-180)agents
(183-186)
mesa_frames/concrete/agents.py (2)
mesa_frames/concrete/polars/agentset.py (3)
agents
(541-542)agents
(545-548)index
(563-564)mesa_frames/concrete/pandas/agentset.py (3)
agents
(434-435)agents
(438-445)index
(460-461)
tests/polars/test_grid_polars.py (1)
tests/pandas/test_grid_pandas.py (6)
get_unique_ids
(19-24)model
(36-43)grid_moore
(46-53)grid_hexagonal
(73-77)grid_moore_torus
(56-63)grid_von_neumann
(66-70)
🪛 Ruff (0.8.2)
tests/pandas/test_grid_pandas.py
8-8: mesa_frames.abstract.agents.AgentSetDF
imported but unused
Remove unused import: mesa_frames.abstract.agents.AgentSetDF
(F401)
mesa_frames/abstract/space.py
57-57: pandas
imported but unused
Remove unused import: pandas
(F401)
66-66: mesa_frames.types_.AgnosticIds
imported but unused
Remove unused import: mesa_frames.types_.AgnosticIds
(F401)
mesa_frames/concrete/pandas/agentset.py
121-121: Yoda condition detected
Rewrite as agents.index.name == "unique_id"
(SIM300)
mesa_frames/concrete/polars/agentset.py
62-62: uuid
imported but unused
Remove unused import: uuid
(F401)
63-63: warnings
imported but unused
Remove unused import: warnings
(F401)
220-220: Within an except
clause, raise exceptions with raise ... from err
or raise ... from None
to distinguish them from errors in exception handling
(B904)
tests/polars/test_grid_polars.py
975-978: Use a single if
statement instead of nested if
statements
Combine if
statements using and
(SIM102)
1312-1315: Use a single if
statement instead of nested if
statements
Combine if
statements using and
(SIM102)
🪛 GitHub Actions: build
tests/pandas/test_grid_pandas.py
[error] 700-700: ValueError: Some agents are not in the model
[error] 744-744: ValueError: Some agents are not in the model
[error] 812-812: AssertionError
[error] 888-888: ValueError: Some agents are not in the model
[error] 932-932: ValueError: Some agents are not in the model
[error] 1000-1000: ValueError: Some agents are not present in the model
[error] 1199-1199: ValueError: Some agents are not present in the model
[error] 1249-1249: AssertionError
[error] 1277-1277: AssertionError
[error] 1296-1296: AssertionError
🔇 Additional comments (59)
mesa_frames/concrete/pandas/mixin.py (1)
54-56
: Formatting improvement in docstring.The removal of the
# noqa: D205
directive suggests that the docstring now complies with style guidelines without needing to suppress linting warnings.tests/pandas/test_grid_pandas.py (2)
3-3
: Added import for Polars library.This import supports the new
get_unique_ids
function that uses Polars Series concatenation.
19-25
: New helper function to retrieve unique agent IDs.The
get_unique_ids
function aggregates unique IDs from different agent types in the model, supporting the automatic ID generation feature. This function is used throughout the test file to replace hardcoded agent IDs.tests/polars/test_agentset_polars.py (2)
72-91
: Good test coverage for rejecting user-defined unique IDs.The new test
test_add_with_unique_id
properly verifies that the system rejects attempts to add agents with predefined IDs, which is a key requirement for the automatic ID generation feature. The test covers all input types: DataFrame, list, and dictionary.
44-44
: Good adaptation of tests to use dynamic agent IDs.The code now properly uses
agents["unique_id"]
with array indexing to select specific agents, replacing hardcoded ID references. This approach aligns with the automatic ID generation feature.Also applies to: 54-56
mesa_frames/concrete/polars/mixin.py (2)
61-65
: Added support for uint64 data type.The addition of "uint64" mapping to
pl.UInt64
in the_dtypes_mapping
dictionary enhances type handling capabilities for agent IDs, which are now stored as unsigned 64-bit integers. This change aligns with the automatic ID generation feature.
615-615
: Improved robustness of series constructor.Converting
data
to a list before passing it to the Polars Series constructor ensures consistent behavior regardless of the iterable type provided. This makes the method more robust and prevents potential issues with different iterable types.mesa_frames/abstract/space.py (1)
1174-1174
: Good change: Using uint64 for agent IDsChanging the agent_id data type to
uint64
aligns with the PR objective of automating unique ID generation. Unsigned integers are appropriate for IDs since they only need non-negative values, and using 64 bits provides a large range (0 to 2^64-1) for potential IDs.tests/test_agents.py (4)
99-105
: Good update: Using dynamic references to agent IDsThe test now references agent IDs dynamically through
agentset_pandas["unique_id"][0]
instead of hardcoded values. This makes the test more robust when agent IDs are auto-generated rather than user-defined.
159-160
: Update to support autogenerated IDsThe assertion now correctly checks that ID 0 is not in the agents collection, which aligns with the PR's goal of automatically generating unique IDs rather than using sequential integers starting from 0.
270-271
: Consider updating comment to reflect changesThis comment refers to converting to a Python list, which is now necessary because both Pandas and Polars implementations need to access the first element consistently.
323-325
: Consistent with autogenerated ID approachThe test now correctly checks that ID 0 is not in the agents collection and generates an appropriate exception when trying to remove it. This aligns with the PR's goal of automatically generating unique IDs.
tests/pandas/test_agentset_pandas.py (12)
14-16
: Good simplification: Removed index parameterRemoving the index parameter from the constructor aligns with the PR's goal of automating unique ID generation. The simplification of
starting_wealth
initialization is appropriate as IDs will now be generated automatically.
28-28
: Updated initialization to support autogenerated IDsThe constructor call for
ExampleAgentSetPandas
has been updated to remove the index parameter, which is consistent with the PR's goal of automating ID generation.
38-38
: Updated initialization consistent with first fixtureGood consistency - both fixtures have been updated to use the new constructor signature without an index parameter.
49-51
: Using unique_id attribute for agent referencesThe code now correctly references agent IDs through
fix1_AgentSetPandas["unique_id"][:2]
instead of using hardcoded indices, making the test more robust.
66-78
: Good test: Validating that manual ID setting is preventedThis new test verifies that the system properly rejects attempts to add agents with predefined IDs, which aligns with the PR objective of automating unique ID generation. The test checks both list and dictionary inputs.
89-91
: Simplified test assertionsThe test now focuses on verifying the length of the agent collection after adding without checking specific ID values, which is appropriate for the autogenerated ID approach.
94-96
: Simplified test assertionsSimilar to the previous change, this test now focuses on verifying the length without depending on specific ID values, which is appropriate for autogenerated IDs.
101-102
: Simplified dictionary input for add methodThe test now adds agents using a simpler dictionary without specifying unique_id values, which aligns with the PR's goal of automating ID generation.
111-112
: Good update: Using dynamic references to agent IDsThe test now references agent IDs dynamically through
agents["unique_id"][0]
instead of hardcoded values, making it more robust with autogenerated IDs.
115-115
: Using dynamic references for agent IDsContinuing the pattern of using dynamic ID references rather than hardcoded values, which supports the PR's objective.
183-187
: Improved explanation and reindexingAdded a helpful comment explaining that the do method doesn't guarantee order preservation and correctly reindexes the result to ensure consistent ordering for test assertions.
296-303
: Updated test for agent concatenationThe test now correctly verifies that when adding two agent sets, the resulting agent set contains IDs from both source sets, using dynamic ID references rather than hardcoded values.
mesa_frames/concrete/pandas/agentset.py (7)
56-56
: Import usage is valid.
Thewarnings
import is indeed used to emit a deprecation warning, so there's no issue here.
114-141
: Add method signature update and unique ID checks look correct.
AddingAgentSetDF
to the accepted types aligns with the PR objective, and preventing user-definedunique_id
columns or keys helps maintain automatically generated IDs. The condition is logically sound.🧰 Tools
🪛 Ruff (0.8.2)
121-121: Yoda condition detected
Rewrite as
agents.index.name == "unique_id"
(SIM300)
146-148
: Verify necessity of handling Polars DataFrame in a Pandas class.
Although checking for a PolarsDataFrame
is not harmful, it's unclear if it's ever expected here, given the function signature. Consider removing or explaining this branch if it’s not truly needed.
152-159
: Mixed usage of Polars references in a Pandas class.
The logic referencingpl.Series
for_mask
andoriginally_empty
may be bridging code, but please confirm that it’s intentional and consistent with the design.
213-214
: Double-check usage of a Polars DataFrame in set operation.
Lines handle Polarsvalues
in a Pandas-basedset(...)
; confirm if passing a PolarsDataFrame
is a legitimate scenario or an edge case that should be disallowed.
407-415
: Uniform unique ID generation aligns with PR goals.
Generatingnp.uint64
IDs in debug mode ensures collisions are tested. This approach seems solid and consistent with the Polars counterpart.
449-449
: Conditional active agents return is reasonable.
Falling back to the whole DataFrame if the mask is empty matches the documented behavior.mesa_frames/concrete/polars/agentset.py (7)
67-67
: ShapeError import is justified.
This import is required for catching column shape issues in theset
method.
107-159
: Add method modifications: consistent with unique ID enforcement.
Rejecting a user-definedunique_id
ensures the PR’s automatic ID assignment strategy. The diagonal relaxation inpl.concat
properly combines data.
175-176
: Collection-based contain checks.
Using an unsigned integer series for ID checks aligns with the new ID type.
213-214
: Accessing values asto_series()
is valid.
This approach helps unify the assignment logic regardless of input type.
239-245
: Conditional unique_id column injection looks good.
Generating new IDs if missing helps maintain consistency with automated ID logic.
250-250
: Updating mask with new indices is correct.
The call to_update_mask
ensures the newly added agents are active if needed.
485-493
: Polars unique ID generation method is consistent with the Pandas counterpart.
Using large ranges in debug mode for collision testing is a solid approach.mesa_frames/concrete/agents.py (10)
51-51
: Importing NumPy is appropriate.
It’s used to handlenp.uint64
checks, aligning with the PR’s automated ID design.
86-86
: Switch to UInt64 for_ids
.
This is consistent with the PR’s switch to unsigned IDs across the codebase.
114-116
: Appending new agent sets and their IDs.
Concatenating each agentset’sunique_id
properly extends the global IDs.
128-128
: Checking fornp.uint64
.
Validates the type more precisely than a generic integer check.
141-141
: Collection-based ID containment.
Usingpl.Series(..., dtype=pl.UInt64).is_in(...)
ensures consistent ID comparison.
213-213
: Handling single ID removal as(int, np.uint64)
.
This is a clear improvement that accounts for both regular int and the new uint64.
225-232
: Building removed IDs withpl.UInt64
.
This ensures the removed set is handled consistently with the rest of the code.
240-241
: Extracting and rewrapping IDs aspl.Series (uint64)
.
This step maintains consistent ID typing during removals.
357-358
: Presence DataFrame updated withpl.UInt64
schema.
Ensures that ID presence checks align with the newly introduced ID type.
361-361
: Enforcing unsigned type for incoming agentset indexes.
Casting agentset indexes topl.UInt64
unifies the ID type across the system.tests/polars/test_grid_polars.py (11)
5-5
: LGTM: Added necessary imports for the new unique ID handlingThe imports for
assert_frame_equal
frompolars.testing
andget_unique_ids
from the pandas test module are appropriate for the changes being implemented throughout this file.Also applies to: 12-12
42-43
: LGTM: Good refactoring to use dynamic IDsReplacing hardcoded agent IDs with dynamic ID retrieval via
get_unique_ids
makes the tests more robust and consistent with the new unique ID generation system.Also applies to: 52-53, 62-63, 69-70
145-145
: LGTM: Properly updated test assertions to use dynamic IDsThe test assertions have been updated to use dynamically retrieved agent IDs, ensuring that tests remain valid with the new unique ID generation system.
Also applies to: 166-176, 178-180, 204-205, 251-254
638-641
: LGTM: Proper agent movement with dynamic IDsAgent movement operations throughout the test now use dynamically retrieved IDs, maintaining consistency with the new unique ID generation approach.
Also applies to: 732-734, 750-753
645-655
: LGTM: Updated assertion checks to use dynamic IDsThe
assert_frame_equal
checks now properly validate DataFrames containing dynamically generated agent IDs. The implementation maintains test integrity while accommodating the new unique ID system.Also applies to: 673-679, 683-693, 697-707, 715-725, 736-746, 755-766
811-812
: LGTM: Consistent ID usage across agent operationsAll agent operations (move, place, remove) have been updated to use dynamic IDs retrieved via
get_unique_ids
, making the test suite consistent with the automated ID generation feature.Also applies to: 865-866, 897-898, 909-910, 914-914, 932-932, 969-970, 974-974, 992-992
814-820
: LGTM: Updated DataFrame assertions to use dynamic IDsAll DataFrame assertions now correctly compare actual results against expected values using dynamically generated IDs. This ensures tests remain valid with the new automatic ID generation system.
Also applies to: 830-840, 850-860, 883-893, 899-905
1050-1066
: LGTM: Updated place_agents tests to use dynamic IDsThe tests for the place_agents functionality have been properly updated to use dynamically generated IDs, maintaining test coverage while accommodating the new ID system.
Also applies to: 1084-1170, 1227-1240
1401-1462
: LGTM: Updated remove_agents tests to use dynamic IDsAll agent removal tests now correctly use dynamic IDs from the
get_unique_ids
function, ensuring proper test functionality with the new ID system.
1588-1589
: LGTM: Updated swap_agents tests to use dynamic IDsThe swap_agents tests now correctly retrieve and use dynamic agent IDs, maintaining test functionality with the new unique ID generation system.
Also applies to: 1599-1607, 1608-1643
1671-1701
: LGTM: Updated property tests to use dynamic IDsThe property tests for agents and cells have been updated to properly use dynamic IDs, ensuring that all assertions work correctly with the new ID generation system.
Also applies to: 1719-1748
Sorry @luca-patrignani can you explain better what you mean? remember that polars has a dtype pl.UInt64 that can be specified explicitly in the constructor. |
My bad, that piece is completely useless. I am removing it. |
for more information, see https://pre-commit.ci
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #110 +/- ##
=======================================
Coverage ? 92.19%
=======================================
Files ? 11
Lines ? 1692
Branches ? 0
=======================================
Hits ? 1560
Misses ? 132
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost there @luca-patrignani!
This is not an easy task. Check my review comments...
@@ -95,12 +98,12 @@ def __init__(self, model: "ModelDF") -> None: | |||
The model that the agent set belongs to. | |||
""" | |||
self._model = model | |||
self._agents = pl.DataFrame(schema={"unique_id": pl.Int64}) | |||
self._agents = pl.DataFrame() | |||
self._mask = pl.repeat(True, len(self._agents), dtype=pl.Boolean, eager=True) | |||
|
|||
def add( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
def add( | |
agents: pl.DataFrame | Sequence[Any] | dict[str, Any], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You cannot add agents from another AgentSetPolars
@@ -188,7 +196,6 @@ def set( | |||
inplace: bool = True, | |||
) -> Self: | |||
obj = self._get_obj(inplace) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain why you had to make changes to the set method?
This PR aims to address issue #105 by generating automatically the unique_id for each agent and not letting the user assign it.
The proposed solution consists in assigning increasing ids to new added agents.
Need confirmation
Summary by CodeRabbit
New Features
Refactor
Tests
Documentation