Conversation
There was a problem hiding this comment.
We should find a good pattern and do it across the repos.
A couple of questions:
- What problem is this addressing?
- Why did you need
PYTHONPATH=.for the numerical test and not the other one?
My idea was to not install keras so that you would know that it's using the local source, which is why keras is not in requirements.txt, which is a little weird in some way.
What I didn't realize is that pip install -e . would install keras. But that's solvable.
- We can do
pip uninstall kerasafter - We can do
pip install --no-deps -e .in thekeras_mlxfolder - We can do
pip install -e .in thekerasfolder first so thatkerasis already installed, but that might have some caveats
I'm not a big fan of this
import os, keras; assert os.path.realpath(keras.__file__).startswith(os.path.realpath('../keras'))
I'd rather make sure keras is not installed at all. But again, I'm not sure why you needed PYTHONPATH=.
|
Thanks for the review! The PyPI keras comes from tensorflow. pytest from |
I don't think you need both. The bulleted list was list of options to pick one from. |
|
Ah got it, I read it as steps! Kept just the uninstall, tensorflow already pulls keras in from the requirements before keras-mlx gets installed, so |
tensorflow and keras-mlx both pull keras in from PyPI, and that one has no mlx backend. This uninstalls it after installing keras-mlx, so if anything ends up importing keras from outside the checkout it fails instead of quietly running the wrong one.
numerical_test.pystill needsPYTHONPATH=.since running a script puts its own folder onsys.path, not the cwd.