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

Replace "." with "/" in module_name #2219

Merged
merged 1 commit into from
Sep 11, 2024
Merged

Conversation

mscroggs
Copy link
Contributor

If a "." is used in module_name, this should be replaced by joining a path, as it represents a subdirectory.

Fixes #2218.

Copy link

netlify bot commented Sep 11, 2024

Deploy Preview for maturin-guide ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit adb5657
🔍 Latest deploy log https://app.netlify.com/sites/maturin-guide/deploys/66e1a808309cde00098cc90b
😎 Deploy Preview https://deploy-preview-2219--maturin-guide.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@messense
Copy link
Member

Just to confirm, do it work for module name that doesn't not contain .?

@mscroggs
Copy link
Contributor Author

Just to confirm, do it work for module name that doesn't not contain .?

Good question. I'll make up a local package now and check

@mscroggs
Copy link
Contributor Author

Just to confirm, do it work for module name that doesn't not contain .?

Picking from the examples in the README, I was able to build and install https://github.com/webonnx/wonnx with this branch

@messense
Copy link
Member

Unfortunately wonnx won't trigger that code path because it does not need to bundle external libs:

$ unzip -l wonnx-0.5.1-cp37-abi3-manylinux_2_17_x86_64.manylinux2014_x86_64.whl
Archive:  wonnx-0.5.1-cp37-abi3-manylinux_2_17_x86_64.manylinux2014_x86_64.whl
  Length      Date    Time    Name
---------  ---------- -----   ----
     1567  09-30-2023 09:12   wonnx-0.5.1.dist-info/METADATA
      129  09-30-2023 09:12   wonnx-0.5.1.dist-info/WHEEL
      103  09-30-2023 09:12   wonnx/__init__.py
 15338144  09-30-2023 09:12   wonnx/wonnx.abi3.so
      354  09-30-2023 09:12   wonnx-0.5.1.dist-info/RECORD
---------                     -------
 15340297                     5 files

@mscroggs
Copy link
Contributor Author

Just to confirm, do it work for module name that doesn't not contain .?

Picking from the examples in the README, I was able to build and install https://github.com/webonnx/wonnx with this branch

.. although I've just realised that that was useless as it doesn't have any shared objects

@mscroggs
Copy link
Contributor Author

Good news: I locally built ndelement with module-name updated to not have a dot, and it built, installed an imported without errors

Copy link
Member

@messense messense left a comment

Choose a reason for hiding this comment

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

Thanks!

@messense messense merged commit 1cd1140 into PyO3:main Sep 11, 2024
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cannot open shared object file: No such file or directory
2 participants