projectmesa / mesa-examples

Seminal agent-based models developed using Mesa
Other
105 stars 123 forks source link

fix: Fallback to None when attribute is missing #90

Closed rht closed 5 months ago

Corvince commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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

EwoutH commented 5 months ago

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

rht commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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"