Recreate add_agent!(agent,...) + new constructor for agents

[Tortar]

Closes #945

[codecov-commenter]

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (433faa4) 92.18% compared to head (ec31ee1) 92.15%.
Report is 6 commits behind head on main.

❗ Current head ec31ee1 differs from pull request most recent head 53bc394. Consider uploading reports for the commit 53bc394 to get more accurate results

Files	Patch %	Lines
src/core/agents.jl	80.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #946      +/-   ##
==========================================
- Coverage   92.18%   92.15%   -0.04%     
==========================================
  Files          33       33              
  Lines        2330     2344      +14     
==========================================
+ Hits         2148     2160      +12     
- Misses        182      184       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Tortar]

ok, I realized we only need to reabilitate add_agent_pos!, the other usages are a bit superfluous with the other add_agent! methods in place. I also re-added add_agent_single! because it is a bit more useful as a function.

In general, add_agent_pos! is an unfortunate name nonetheless because it could well be just add_agent!, which would be better for example with a nothing space, and it seems the correct name in any case. Unfortunately we used this version for adding the agent to a random position in the past, and so changing it to instead add it at the own position of the agent is a breaking change. What I'm thinking is that we can send to the direct users of add_agent_pos! a future warning saying that this function will be renamed add_agent! (maybe in 6.2?). Or keep it like this, even if it doesn't seem the right name. What do you think?

I also added the constructors accepting as the first argument the model instance so that it creates agents the way it should. I will document it in the agent macro docstring.

[Datseris]

In general, add_agent_pos! is an unfortunate name nonetheless because it could well be just add_agent!, which would be better for example with a nothing space, and it seems the correct name in any case. Unfortunately we used this version for adding the agent to a random position in the past, and so changing it to instead add it at the own position of the agent is a breaking change. What I'm thinking is that we can send to the direct users of add_agent_pos! a future warning saying that this function will be renamed add_agent! (maybe in 6.2?). Or keep it like this, even if it doesn't seem the right name. What do you think?

Ι agree with you. This is an unfortunate naming. My worry however is that this function is so fundamental, and so integrated into the package and all its usage, that changing would be just too much.

What I think is the best solution is to find a better name alltogether for the function. How about add_agent_own_pos? This signifies that the agent is added to its own position.

I believe that on average it is more common that the user wants agents added to random positions rather than a specified position hence I would argue that the less verbose function add_agent should stay as is and do the random position unless specified otherwise.

What do you think of that?

This means that add_agent_pos is deprecated and throws a warning for the new function name.

[Tortar]

Thought about this a bit, and I agree with you that add_agent_own_pos! is a little bit better so I guess it's okay to have both add_agent!(agent, [, pos], model) and add_agent_own_pos!(agent, model). Will update this PR in this direction.

[Datseris]

@Tortar what is the status here? is this PR ready for review?

[Tortar]

Need still to finish this, I'll do it today

[Tortar]

Should be okay now!

[Tortar]

ok very good :D, same tests failing as in #875, locally everything works as in the other PR so we can merge both, but something is strange on Github and we should try to understand what

[Datseris]

Yes. Just make sure you add the changelog entry for the deprecation and then you can merge. (also couple of minor comments)