Skip to content

Conversation

@tpike3
Copy link
Member

@tpike3 tpike3 commented Jul 2, 2023

@tpike3 tpike3 added this to the Mesa 2.0 (Wellton) milestone Jul 2, 2023
@codecov
Copy link

codecov bot commented Jul 2, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (4e36a6a) 79.96% compared to head (fea5afe) 79.96%.

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

Additional details and impacted files
@@           Coverage Diff           @@
##             v2.0    #1727   +/-   ##
=======================================
  Coverage   79.96%   79.96%           
=======================================
  Files          18       18           
  Lines        1178     1178           
  Branches      220      220           
=======================================
  Hits          942      942           
  Misses        202      202           
  Partials       34       34           

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

HISTORY.rst Outdated
* space: change `coord_iter` to return `(content,(x,y))` instead of `(content, [x],[y])`; this reduces known errors of scheduler to grid mismatch #1566, #1723
* space: change NetworkGrid `get_neighbors` to `get_neighborhood`; improves performance #1542
* space: raise exception when pos is out of bounds in `Grid.get_neighborhood` #1524
* space: remove deprecations (`find_empty` -> `move_to_empty`, num_agents -> removed param from `move_to_empty`, `position_agent` -> `place_agent`, `neighbor_iter` -> `iter_neighborhood`); complete deprecated function removal #1520, #1687, #1688
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find this one too cryptic.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line 18? what if it reads as

space: remove deprecations (use move_to_empty instead of find_empty, use position_agent instead of place_agent use iter_neighborhood instead of neighbor_iter; move_to_empty no longer has the parameter num_agents (int)); complete deprecated function removal #1520, #1687, #1688

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could format them as a list (nested). This should be considered as a recipe for migration. As such, you should say the obsolete function first, then the replacer, because people are going to follow it step by step.

Copy link
Member Author

@tpike3 tpike3 Jul 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So...

space: remove deprecations [(find_empty() convert to move_to_empty()), (remove num_agents as a parameter from move_to_empty()), (position_agent() convert to place_agent()), (neighbor_iter() convert to iter_neighborhood()]; complete deprecated function removal #1520, #1687, #1688

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uh, I meant for it to be read by humans, not LLMs.
I meant list in a list:

  • Remove deprecated functions.
    • find_empty: convert this to move_to_empty
    • position_agent: convert this to place_agent

I think it is plausible that the LLMs can automate the migration of existing code, if they are fed the old API and new API data, and the migration guide.

@rht
Copy link
Contributor

rht commented Jul 8, 2023

Why is the target branch v2.0? Shouldn't it be main?

@tpike3
Copy link
Member Author

tpike3 commented Jul 8, 2023

Why is the target branch v2.0? Shouldn't it be main?

@jackiekazil told me to do to v2.0, if I remember correctly that then makes the release easier.

@tpike3
Copy link
Member Author

tpike3 commented Jul 8, 2023

Forgot to update the date

* `position_agent()`: convert this to `place_agent`
* `neighbor_iter()`: convert this to `iter_neighborhood()`
* batchrunner: remove deprecations #1627
* `batchrunner()` and `batchrunnerMP()`: convert these to `batch_run()`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aren't those supposed to be BatchRunner and BatchRunnerMP classes, not functions?

* `neighbor_iter()`: convert this to `iter_neighborhood()`
* batchrunner: remove deprecations #1627
* `batchrunner()` and `batchrunnerMP()`: convert these to `batch_run()`
* visualization: remove `UserSettableParameter`; easier visualization creation #1693
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is too terse. I will have no idea what to replace UserSettableParameter with.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put in some changes feel free to edit as well so we can get this out the door.

@jackiekazil jackiekazil changed the base branch from v2.0 to main July 13, 2023 02:43
@jackiekazil
Copy link
Member

@tpike3, @rht I changed the target branch to Main -- rht is right. We never updated v2.0.

#1726 was merged.

@tpike3 is everything tagged with the milestone?
And can you do final updates on the history?
I can deploy Thursday or Friday... but I have a slight preference for Saturday because I have more time if there are any issues. :-)

@tpike3
Copy link
Member Author

tpike3 commented Jul 13, 2023

@tpike3, @rht I changed the target branch to Main -- rht is right. We never updated v2.0.

#1726 was merged.

@tpike3 is everything tagged with the milestone? And can you do final updates on the history? I can deploy Thursday or Friday... but I have a slight preference for Saturday because I have more time if there are any issues. :-)

Changed the date to Saturday the 15th

@tpike3 tpike3 closed this Jul 13, 2023
@jackiekazil
Copy link
Member

jackiekazil commented Jul 13, 2023 via email

@tpike3
Copy link
Member Author

tpike3 commented Jul 14, 2023

Is there a reason this was closed?

Apologies was rushing this was superseded by #1731 -- merge into main instead of v2.0

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