-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Type Enhancement for Func Transforms and Bug Fix #3003
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
base: main
Are you sure you want to change the base?
Conversation
|
Also, |
setup.py
Outdated
| extdir = ext_fullpath.parent.parent.resolve() | ||
| build_temp = Path(self.build_temp) / ext.name | ||
| env = os.environ.copy() | ||
| env["PYTHONPATH"] += f":{extdir}" |
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.
I'm not sure if this actually fixes the problem you saw, the exact path has already been passed in the stubgen target:
Lines 35 to 40 in d2bef3c
| # Run stubgen -m mlx.core -i python -p _stub_patterns.txt -o python/mlx | |
| RECURSIVE | |
| MODULE | |
| "mlx.core" | |
| PYTHON_PATH | |
| "${CMAKE_CURRENT_SOURCE_DIR}/.." |
Can you share the exact command that you used?
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.
the command is pip install git+https://github.com/ml-explore/mlx.git -vv and pip install -vv git+https://github.com/XXXXRT666/mlx.git@type-fix
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.
CMAKE_CURRENT_SOURCE_DIR = ~/mlx/python/src in my environment, but mlx.core is generated in ~/mlx/build/lib.macosx-11.0-arm64-cpython-310/mlx/core.cpython-310-darwin.so
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.
Thanks a lot for the information! Can you check if #3009 fixes the error for you?
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.
Thanks a lot for the information! Can you check if #3009 fixes the error for you?
It doesn't fixed the error, I think the correct path is $<TARGET_FILE_DIR:core>/.. as <TARGET_FILE_DIR:core> is the path to the directory of the target .so file, which is ~/mlx/build/lib.macosx-11.0-arm64-cpython-310/mlx
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.
Oh I only tested local install and I guess it searched from the other path, thanks for testing. Do you mind checking the updated PR again? This time it passes the remote install for me. pip install -v git+https://github.com/zcbenz/mlx.git@stubgen-python-path
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.
Yes no error occurs, stubs are generated correctly
I didn't found it in the repo, maybe it's has been modified |
Sorry my bad, it was already removed in #2949, the website has not been updated yet. |
This reverts commit 887a6ca. Revert
zcbenz
left a comment
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.
I don't have much experience with python typing but after reading the docs I think this is just standard usage. Still needs a check from @awni .
Proposed changes
Type Hints After This PR:
Checklist
Put an
xin the boxes that apply.pre-commit run --all-filesto format my code / installed pre-commit prior to committing changes