-
Notifications
You must be signed in to change notification settings - Fork 928
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
Importable examples #2381
Importable examples #2381
Conversation
Performance benchmarks:
|
Awesome, thanks for working on this! Let me know when you need a review or something else. |
aff5ea7
to
706f03d
Compare
@EwoutH (and others) this is now ready for review! For now only the basic examples are covered, until the others are updated. |
Performance benchmarks:
|
I have now fixed the visualisation tutorial as well. I originally thought we have to fix that in mesa-examples, but we dont so I included it here /edit *tried to fix it |
@quaquel Do you have any idea why this might be failing on RTD? Is rtd doing something special or building mesa differently? |
that doesn’t look like the tutorial uses the version from this PR, could that be the culprit? |
Ah, yes, of course. I thought there is a problem with the version on RTD, but the error is of course inside the notebook. Thanks! So theoretically this will solve itself with the next pre-release. Do you think that is sufficient? |
Yeah for now I do think so. It’s something we need to keep in mind while developing these notebooks. |
The error persists in my work on docs so there is more going on here than just an incorrect version of MESA being installed. I'll try to find a wor arround for now because it gets in the way of testing examples in docs, but we need to figure out what is going on here and properly fix it. |
Actually, the relative imports I introduced here were a bad idea. If you copy the files to a different folder it will not work due to "cannot import relative without a known parent module", i.e. you would additionally need an (empty) init.py file. I don't know what the best solution is here. |
Yeah, when porting the advanced examples over I now also get constantly:
We need to take another look at this. |
You are right. Just to write down the requirements:
Both work with |
The absolute brute force is: try:
# Attempt to import when running as part of the package
from .model import MODELNAME
except ImportError:
# Fallback to import when running directly
from model import MODELNAME The alternative is that we drop requirement 2, and require full-path imports if not from the example folder itself. Like: from mesa.examples.advanced.epstein_civil_violence.agents import Citizen, Cop
from mesa.examples.advanced.epstein_civil_violence.model import EpsteinCivilViolence |
I blew about half of my weekly ChatGPT o1-preview budget on this, and we keep coming down to one of three solutions:
|
when you say running the script, do you mean app.py? and do you mean running from the command line? |
Here is another requirement I would like to see fullfilled: Copy the example folder somewhere else (e.g. pycafe, user project) and still be able to use it. Therefore we can't use 3). Since we (currently) only run into this when we run /edit btw the relative imports work when running on py.cafe |
Also posted on discuss.python.org: https://discuss.python.org/t/making-python-scripts-work-both-as-imports-and-executables-is-there-a-clean-solution/68766/1. If mesa is installed (with pip) or just cloned also matters a lot. |
Would |
To move this forward I would suggest changing the imports to absolute imports for now (import mesa.examples.basic.boltzmann_wealth.model). That allows running with solara and being importable. The only thing that's not without additional work is copying the examples into other folders. But that's probably the least important workflow and we can still think about a way to copy/download models as a user (and adjusting imports on the way) |
Got a few suggestions at discuss.python.org. |
This makes the basic examples importable. You can now do
from mesa.examples.basic import BoltzmannWealthModel
.Tried a few hours to convince hatch to link examples into mesa. Got it working eventually, but it broke editable installs. Since editable installs are the best feature since bread and butter the error there pointed me towards simply specifiing
__path__
in a stub example module.This PR only works on basic examples, because the advanced ones haven't been updated yet to a better standard.