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

Fix for embedded Python interpreter issues on macOS #755

Merged
merged 2 commits into from
Mar 20, 2024

Conversation

robertbartel
Copy link
Contributor

Fixes issues described in #628.

Moving to a newer version of pybind11 that has this issue fixed. Also modifying the order in which items (e.g., virtual environment directories) are added to the interpreter system path, but only on macOS.

These changes allow for successful and correct loading of numpy 1.26.3 from the virtual environment (as was used during build system generation), where previously 1.26.2 was being loaded from the system-level installed packages.

@robertbartel robertbartel added the bug Something isn't working label Mar 6, 2024
@PhilMiller
Copy link
Contributor

Why the macOS change? The reasoning for the pybind11 update is quite clear, but that one's not. Especially why insert at 1 rather than 0

@PhilMiller
Copy link
Contributor

Ping. What's the motivation for the macOS path order difference?

@robertbartel
Copy link
Contributor Author

@PhilMiller, basically the reason was that this is intended to fix an issue that seemingly only affects MacOS. This is described in #628, and I experienced the issue myself. I suppose I am assuming here that Linux environments truly aren't having this problem (I haven't actually confirmed this myself), though, even beyond Matts original description, I'd expect to have seen a lot more attention on the problem if Linux were also impacted.

Given that the issue is a MacOS-only problem, I was attempting to minimize the impact of the changes. But we don't have to if consistency seems more important.

@robertbartel
Copy link
Contributor Author

As for why to insert at 1, it was because the path at index 0 was the current directory. I wanted to move the virtual environment packages to a higher priority when searching for imports, but felt that searching in the current directory probably should still be done first.

@robertbartel
Copy link
Contributor Author

Also, to clarify something I might have earlier missed was being asked ...

Moving to a new version of pybind11 was not enough to resolve the problem. Both a newer pybind11 and modifications to adding directories to the interpreter's system path were necessary to get the embedded interpreter to import the correct numpy package.

Copy link
Contributor

@PhilMiller PhilMiller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit uneasy about the inconsistency, but this does empirically fix things for me on a Mac that I'm not experiencing on Linux, so it's maybe best to limit the change's impact for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants