Skip to content

[Enhancement]: Consider using a key-based structure for agentsets instead of list in AgentsDf #146

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

Open
Ben-geo opened this issue Apr 9, 2025 · 2 comments
Labels
enhancement Improvements to existing features or performance. good first issue Good for newcomers

Comments

@Ben-geo
Copy link
Collaborator

Ben-geo commented Apr 9, 2025

🤔 Problem Description

Currently, agentsets are stored in a list (e.g., self.agentsets[0] for sheep, self.agentsets[1] for wolves in wolf-sheep model ), which relies on indexing. While functional, this can be slightly confusing, especially in models with multiple agent types.

Accessing agentsets by name (e.g., "sheep" or "wolves") instead of an index would make the code more readable, less error-prone, and easier to maintain

💡 Proposed Solution

One possibility is to use a key-value structure for storing agentsets—such as a dictionary— open to other approaches as well. The key idea is to allow referencing agentsets with descriptive identifiers rather than implicit positions.


self.agentsets = {
    "sheep": sheep_agentset,
    "wolves": wolf_agentset
}

maybe we can pass in the name when agentsets are added

This could make code that works with specific agent types more intuitive, while still allowing for easy iteration using .values().

Curious if there was a specific reason behind the current list-based design—happy to hear thoughts or alternative ideas!

🔄 Alternatives Considered

No response

➕ Additional Context

No response

@Ben-geo Ben-geo added feature New functionality added to the project. triage Needs initial review and categorization labels Apr 9, 2025
@adamamer20
Copy link
Member

That's a wonderful idea—it makes AgentsDF much more explicit in its purpose and easier to use overall. I love the direction you're going with this!

A couple of things we might want to think about:

  1. Naming the agentset key: Should we enforce a default name or let users define it themselves? Having a clear convention here would help with consistency.

  2. Making access easier: We could implement a few dunder methods to make iteration and indexing more intuitive. For example:

    class AgentsDf:
        def __init__(…):
            # after building self._agentsets = {"sheep":…, "wolves":…}
            pass
    
        def __iter__(self):
            return iter(self._agentsets.values())
    
        def __len__(self):
            return len(self._agentsets)
    
        def __getitem__(self, key):
            # support both integer and string lookup
            if isinstance(key, int):
                return list(self._agentsets.values())[key]
            return self._agentsets[key]

    This would allow users to write cleaner and more Pythonic code, like:

    for agentset in agents_df:
        …  # implicitly iterates over values
    
    sheep, wolves = agents_df  # tuple-unpacks in insertion order
    wolves        = agents_df["wolves"]
    first_set     = agents_df[0]

@adamamer20 adamamer20 added enhancement Improvements to existing features or performance. good first issue Good for newcomers and removed feature New functionality added to the project. triage Needs initial review and categorization labels Apr 17, 2025
@Ben-geo
Copy link
Collaborator Author

Ben-geo commented Apr 18, 2025

  1. Naming the agentset key: Should we enforce a default name or let users define it themselves? Having a clear convention here would help with consistency.

so I was thinking a bit more about this and came up with the below approaches

Approach 1

it would be be better if we could use type[AgentSetDF] like the function


def agentsets_by_type(self) -> dict[type[AgentSetDF], Self]:
        """Get the agent sets in the AgentsDF grouped by type.

        Returns
        -------
        dict[type[AgentSetDF], Self]
            A dictionary mapping agent set types to the corresponding AgentsDF.
        """

        def copy_without_agentsets() -> Self:
            return self.copy(deep=False, skip=["_agentsets"])

        dictionary: dict[type[AgentSetDF], Self] = defaultdict(copy_without_agentsets)

        for agentset in self._agentsets:
            agents_df = dictionary[agentset.__class__]
            agents_df._agentsets = []
            agents_df._agentsets = agents_df._agentsets + [agentset]
            dictionary[agentset.__class__] = agents_df
        return dictionary

maybe this can be the default way and let users change the name if they pass a specific argument .
however in this case #124 will not be necessary as ( link to comment since the description of that issue was terrible 😂)
by default we won't allow multiple agentsets to have the same type.

cons :

  • users will have to redefine the agentsets even if they have the same properties and just behave differently within the models

pros :

  • very straightforward

personally I like this approach the best since I think its very intuitive.

Approach 2

make the default key the same as index if no name is passed, this will give users the ability to name if they want to ( and we can even add the ability to change the name later as well.

2. Making access easier: We could implement a few dunder methods to make iteration and indexing more intuitive. For example:

I love this
something I would like to consider is converting all the values into list then indexing uses soo much more space right rather than maybe having index to key mapping and accessing it like that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvements to existing features or performance. good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants