-
Notifications
You must be signed in to change notification settings - Fork 880
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
breaking: Remove visualization_old (mesa-viz-tornado) #2133
Conversation
This fixes the tests. |
Performance benchmarks:
|
cfd7efa
to
f3e1d2f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally, I'm going to approve this. Let's cut the cord and focus on the future. The new space needs some work (like support for Cell spaces and PropertyLayers) but it's the future.
I would like more maintainers to pitch in and/or approve (at least a second) before merging though, since this is a big change.
To be clear -- Does this do the following?
Questions --
|
It does 2 items except for adding the deprecation warning. The deprecation warning is to be added in the branch https://github.com/projectmesa/mesa/tree/2.3.x-maintenance.
#2091 needs to be merged first, then I rebase this PR, and then it can be merged.
The examples being tested in CI don't have frontend in them. A manual testing would be to test the Solara examples in mesa-examples.
This is for 3.0. |
Without manual changes, they will fail because they still use the |
As I mentioned in our dev meeting, my concern is about what the users need to do for this... So if a user wants to stick to tornado visualization, he/she will not be able to use Mesa's new features? Conversely if a user wants to update Mesa, he/she will need to migrate from tornado to jupyter? I might be wrong but if this is the case, shall we have some kind of migration guide to assist the users' transition, so that they are not confused or frustrated. |
That would be the idea (and I support it). Old visualisation use Mesa 2.3.x, new visualisation for 3.0. We can't support everything forever, we have to move forward. |
@EwoutH, this feels like a clean way to break it. @wang-boyu
We could do a write-up for users and announce it. We need 1 or 2 use cases to create adequate documentation. Maybe we clarify that people can and are welcome to ask for help with conversions? |
I would recommend merging this and then tag a first pre-release (Mesa |
We can use the two examples in the mesa-examples repo:
|
I have merged #2091 and rebased this PR so that it only has 1 relevant commit. |
## Subpackages | ||
|
||
```{toctree} | ||
mesa.visualization_old |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add the new visualisation here? It's nice to have that build in the docs, even if we don't have that much docstring.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That can happen in a separate PR. This PR is rather expensive to maintain/rebase.
I think it should be either one of them, and virus on a network or boid flocker, because the 2 examples you listed have some overlap in the usage of space. Virus on a network uses network visualization, and boid flocker uses continuous space. |
Thanks a lot for all the work! |
Yup for instance NetLogo has transition guide for new releases: https://ccl.northwestern.edu/netlogo/docs/transition.html |
Removing the old, legacy visualisation. It will still be available in 2.3.x. Checkout the Visualization Tutorial (https://mesa.readthedocs.io/en/latest/tutorials/visualization_tutorial.html) to migrate to the new visualisation.
TODO: