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: Correct module naming and registration #23

Merged
merged 2 commits into from
May 4, 2023

Conversation

MarquessV
Copy link
Contributor

Closes #21

As a workaround to pyo3#759, create_init_submodule! updates sys.modules to register a module. sys.modules is a mapping of the fully qualified path to a module, to the module object itself. create_init_submodule! was using the fully qualified path as both the key and the name of the module. This made submodule import paths awkward to work with, especially with tooling, and in some cases it even breaks imports.

For example, quil-py defines a module quil, with submodules expression, instructions, program, and validation.

Using the current release of rigetti-pyo3, notice the repetition of quil.expression when inspecting the quil module. This repetition also breaks the import quil.expression syntax, because the real path is quil.quil.expression but that module is impossible to access with the usual import syntax, since quil.quil.expression is recognized as three modules delimited by a . by Python, but really it's just two modules, "quil" and "quil.expression".

before-fix

After this fix, the quil module works as expected and we can import quil.expression successfully.

after-fix

@MarquessV
Copy link
Contributor Author

MarquessV commented Apr 14, 2023

cargo msrv is failing, but I'm not sure why. It works locally, both directly on my machine and in a linux docker container using the same toolchain as CI.

Copy link
Contributor

@Shadow53 Shadow53 left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks for finding and fixing this.

@MarquessV MarquessV merged commit 31fdcec into main May 4, 2023
@MarquessV MarquessV deleted the 21-fix-module-registration branch May 4, 2023 17:47
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.

Modules get exported with an extra submodule.
2 participants