mesa
mesa copied to clipboard
examples: Rename model.grid to model.space
Fixes #1227.
Ugh, this is actually a breaking change, because HexGridVisualization and CanvasGridVisualization assume model.grid
. Should we introduce this breaking change?
Ugh, this is actually a breaking change, because HexGridVisualization and CanvasGridVisualization assume
model.grid
. Should we introduce this breaking change?
This is a pretty big one where I think the majority of user visualizations will break. I started working through some work arounds but none were suitable, so will think more on it.
Broad strokes I think there is three paths:
- we do it
- we don't do it
- we keep grid for the examples in
HexGridVisualization
andCanvasGridVisualization
as they are grids and change the others which are not grid
There is always room for adding an informative error message, at the right time. In the 2 classes, we check if users have model.grid
defined. If so, we raise an exception telling what is wrong, and recommend people to use model.space
.
I think this is not worth a breaking change. I think we should either
- Do what @rht said, but with a warning or
- Allow both .grid and .space, but think about how to deal with this in the future.
The reason for 2 is that I think depending on an implicit naming scheme is bad practice. We should either somehow enforce a name or leave the choice completely open to the users.
I think this is not worth a breaking change. I think we should either
- Do what @rht said, but with a warning or
- Allow both .grid and .space, but think about how to deal with this in the future.
The reason for 2 is that I think depending on an implicit naming scheme is bad practice. We should either somehow enforce a name or leave the choice completely open to the users.
Concur, and I vote 2 as we have already had issues with implicit naming and my bias would be to leaving the choice to the users
We sidestepped the model.datacollector
naming by introducing model.initialize_datacollector
(which does initial data collection under the hood, where people often forget to do). But we can't do the same with space/grid, because with datacollector, >90% of the case it is mesa.DataCollector
, but with space, it could be SingleGrid
, MultiGrid
, NetworkGrid
, ...
We could just add space
as an additional function parameter to the modules. CanvasGrid currently takes model width and space, but doesn't actually use them, don't know how that evolved. But I think this is a clean solution and should even he doable without breakage.
The width and height could be passed explicitly, but what about https://github.com/projectmesa/mesa/blob/685494d4b022312ebeabf7dd89013b3bdd6526c2/mesa/visualization/modules/CanvasGridVisualization.py#L97 ?
The width and height could be passed explicitly, but what about
https://github.com/projectmesa/mesa/blob/685494d4b022312ebeabf7dd89013b3bdd6526c2/mesa/visualization/modules/CanvasGridVisualization.py#L97
? ( Sorry I realised my comment was a bit confusing. I would just add a
space
parameter (with a space instance) and derive everything needed from that. The width/height is just an oddity i saw when analysing CancasGrid. Just to be clear, a complete example might look something like this
mycanvas = CanvasGrid(grid=mymodel.space, canvas_width=500, canvas_height=500)
Since the examples are now in their own repo this can be closed