Skip to content

fix: Fallback to None when attribute is missing#90

Merged
rht merged 1 commit intomesa:mainfrom
rht:fix-eg
Jan 24, 2024
Merged

fix: Fallback to None when attribute is missing#90
rht merged 1 commit intomesa:mainfrom
rht:fix-eg

Conversation

@rht
Copy link
Copy Markdown
Contributor

@rht rht commented Jan 24, 2024

No description provided.

@Corvince
Copy link
Copy Markdown
Contributor

I think a mroe typical solution would be

agent_reporters={"Wealth": 'wealth'},

This includes the fallback already. But I am approving, you can change it or merge it as is, your decision

@rht rht merged commit b547ee3 into mesa:main Jan 24, 2024
@rht rht deleted the fix-eg branch January 24, 2024 10:50
@rht
Copy link
Copy Markdown
Contributor Author

rht commented Jan 24, 2024

I picked this one because explicit is better than implicit. The API should have made it clear that only one type of agent is the one actually being collected. Otherwise, users will have to learn the API indirectly by observing the data collected when the agent type is Bank.

@EwoutH
Copy link
Copy Markdown
Member

EwoutH commented Jan 24, 2024

I would also support this approach, with something like:

agent_reporters={"Wealth": 'wealth'},  # The Bank doesn't have a "wealth" attribute, so None is collected for it.

@rht
Copy link
Copy Markdown
Contributor Author

rht commented Jan 24, 2024

That still depends on the code comment, not the API itself. A reader of another non-example code would still be confused by the implicitness of {"Name": "attribute"}.

@rht
Copy link
Copy Markdown
Contributor Author

rht commented Jan 24, 2024

Let's just keep it as is, and focus on revamping the data collector for 3.0.

@EwoutH
Copy link
Copy Markdown
Member

EwoutH commented Jan 24, 2024

That's what comments are for right, explaining things that are non-obvious.

@rht
Copy link
Copy Markdown
Contributor Author

rht commented Jan 24, 2024

Then you will have to carry that explanatory comment everywhere whenever the API may return None for the attribute, as a crutch for the API implicitness.

@EwoutH
Copy link
Copy Markdown
Member

EwoutH commented Jan 24, 2024

These are example models, I think we can explain quite a lot more with comments in the code than we would do in other places.

@Corvince
Copy link
Copy Markdown
Contributor

I think someone not so familiar with python has a hard time to understand

lambda x: getattr(x, "wealth", None)

Things you need to know:

  1. What is a lambda function
  2. What is x
  3. What does getattr do
  4. What does the None mean

In contrast with just the attribute name you only need to know that's one way to collect attributes. That's explained in the tutorial.

And I think it's Intuitive to expect a non-existent attribute to return None.

But yeah, hopefully we find a better API for the next data collector

@rht
Copy link
Copy Markdown
Contributor Author

rht commented Jan 24, 2024

And I think it's Intuitive to expect a non-existent attribute to return None.

This is rather specific to JavaScript, where obj.attr may be null and does not raise an error.

@Corvince
Copy link
Copy Markdown
Contributor

Actually it returns undefined, but that's not important.

Depending on your background I agree that object.attribute leads to different ideas of the outcome.

But we are not doing this directly here. We have abstracted away the mechanics. We just raise the expectation that users get the value of the specified attribute. I don't think users would think the API breaks for multi agent models.

So to put the argument on its head. You are right that it's reasonable for some people to expect this not to work. So we could use the example to show that it does indeed "just works"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants