Skip to content
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

Stabilize PropertyLayer #2440

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

EwoutH
Copy link
Member

@EwoutH EwoutH commented Oct 30, 2024

Removes the warning that the PropertyLayer is experimental, and thus stabilize it.

It has been used in example models (projectmesa/mesa-examples#214), integrated in the cell space (#2319) and the visualisation (both standalone (#2336) and as part of cell spaces (#2430)) and in Mesa for almost a year (#1898) without needing API changes.

Note that while the PropertyLayer itself is stable, not all it's integrations might be, like the integration into the still experimental cell space.

Removes the warning that the PropertyLayer is experimental, and thus stabilize it. It has been used in example models, integrated into the visualisation (both standalone and as part of the cell space) and in Mesa for almost a year without needing API changes.

Note that while the PropertyLayer itself is stable, not all it's integration might be, like the integration into the still experimental cell space.
@EwoutH EwoutH added the feature Release notes label label Oct 30, 2024
@EwoutH EwoutH added this to the v3.0 milestone Oct 30, 2024

This comment was marked as off-topic.

@quaquel
Copy link
Member

quaquel commented Oct 30, 2024

We need to resolve the coordinate system discussion raised in #2431 as part of the stabilization.

@EwoutH
Copy link
Member Author

EwoutH commented Nov 1, 2024

What would minimally be done to declare the current functionality stable?

(CC @wang-boyu)

@quaquel
Copy link
Member

quaquel commented Nov 1, 2024

Resolve #2431.

Thinking about this a bit more and looking at the code, I would also move the property layer classes into their own property_layers.py file. Currently, mesa.space is over 1600 lines of code and hard to navigate. A challenge is to figure out how to abstract _PropertyGrid functionality in such a way that it becomes a mixin that can be used with both old style and new style grids. This move is actually needed in preparation of stabilizing new style spaces and ensures that we don't need to break the API in 3.1 or jump through weird hoops to not break it. We could deprecate from mesa.space import PropertyLayer in 3.0, already support from mesa.property_layer import PropertyLayer in 3.0, and make that the default in 3.1.

@EwoutH
Copy link
Member Author

EwoutH commented Nov 1, 2024

Yeah I was also thinking of splitting it out. We could do mesa.spatial maybe for if we want to add more spatial / CRS / geo stuff

@quaquel
Copy link
Member

quaquel commented Nov 1, 2024

Yeah I was also thinking of splitting it out. We could do mesa.spatial maybe for if we want to add more spatial / CRS / geo stuff

I suggest underpromising by calling it property_layerand keeping the idea of integrating where possible stuff from mesa-geo as a possible over-delivery in the longer run. Remember, we have to life with the API choices we make on this for at least MESA 3.

@EwoutH
Copy link
Member Author

EwoutH commented Nov 1, 2024

That's why I was considering a more general name instead of a specific file for one single class.

@quaquel
Copy link
Member

quaquel commented Nov 1, 2024

In my view it would be PropertyLayer and HasPropertyLayers (a refactor of _PropertyGrid that can also be used with newstyle discrete spaces).

@quaquel
Copy link
Member

quaquel commented Nov 15, 2024

Just a quick remark: at the moment, property layers throw user warnings based on the dtype stuff. Its not possible to get rid of those, because of the hardcoded np.float64, this should be changed to float or the test before throwing the user warning should be changed. I prefer the former since for numpy np.float64 and the python built-in float are equivalent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Release notes label
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants