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

add #[pymodule] mod some_module { ... }, v2 #3294

Closed
wants to merge 1 commit into from

Conversation

birkenfeld
Copy link
Member

Implements using #[pymodule] directly on modules.

Allows importing other wrapped items with a #[pyo3] use foo;.

Allows having a #[pymodule_init] function to run arbitrary code after adding all wrapped items.

This is #2367 brought up to date with main.

@birkenfeld
Copy link
Member Author

Unfortunately now test_declarative_module fails because the non-declarative submodule is initialized twice. This assertion probably hadn't been there when you made the original PR?

Copy link
Member

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

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

Brilliant! Thank you for picking this up 🙏

I've put a comment below with a suggestion how we can avoid the test failure.

What do we need to finish here to make this useful? I think as a starting point it's good enough to merge if we can add submodules, classes and free functions with #[pymodule], #[pyclass] and #[pyfunction] items inside of the module. (Add them automatically instead of needing to add them in the #[module_init].)

We can add those to the test of test_declarative_module.

As follow ups we can deprecate #[pyfn] shorthand of functional modules, and maybe even completely deprecate functional modules (as #[module_init] should allow all the same functionality to be replicated).

Comment on lines +86 to +88
pub fn add_to_module(&'static self, module: &PyModule) -> PyResult<()> {
module.add_object(self.make_module(module.py())?.into())
}
Copy link
Member

Choose a reason for hiding this comment

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

I think to avoid the reinitialization we could use a hypothetical self.find_or_make_module() here which uses PyState_FindModule to retrieve the existing module object (if it exists).

I think this exists on all CPython versions, for PyPy it only exists on PyPy 3.9 (with the wrong name) and PyPy 3.10. Probably it'll be good enough to just reinitialize on PyPy 3.8 and older, they're out of support anyway.

I'll send a PR now to fix the PyPy definitions for PyState_FindModule.

Comment on lines +140 to +144
item_fn.attrs.retain(|attr| {
let found = attr.path().is_ident("pymodule_init");
is_module_init |= found;
!found
});
Copy link
Member

Choose a reason for hiding this comment

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

I think this is where we can scan for #[pyclass], #[pyfunction] and #[pymodule].

If the implementation of how it would be cleanest to add these items to the module isn't readily available I'm happy to help figure this out.

Comment on lines +101 to +103
syn::UseTree::Glob(glob) => {
bail_spanned!(glob.span() => "#[pyo3] cannot import glob statements")
}
Copy link
Member

Choose a reason for hiding this comment

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

It would be nice to add a UI test for declarative modules which checks this error.

!found
});
if is_module_init {
ensure_spanned!(pymodule_init.is_none(), item_fn.span() => "only one pymodule_init may be specified");
Copy link
Member

Choose a reason for hiding this comment

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

Similarly a UI test for this error here would be desirable.

} = &mut module;
let items = match content {
Some((_, items)) => items,
None => bail_spanned!(module.span() => "`#[pymodule]` can only be used on inline modules"),
Copy link
Member

Choose a reason for hiding this comment

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

UI test also makes sense for this one, I think.

@holzschu
Copy link

Following the mention in #3382, I hope this is a better place for suggesting that it would be great if PyO3 modules could have m_free and m_clear functions, that can be called when we want to manually clear up the module from memory.
Ideally, these functions would leave the module in a state where it can be reinitialized again.

My motivation is that I'm working with non-standard architectures, where the process continues (so memory is not automatically released) but the interpreter is restarted, so I manually free all modules when the interpreter ends and reinitialize them (I'll be the first to admit that's a niche concern).

Tpt added a commit to Tpt/pyo3 that referenced this pull request Feb 9, 2024
Based on PyO3#2367 and PyO3#3294

Allows to export classes, native classes, functions and submodules and provide an init function

See test/test_module.rs for an example

Future work:
- update examples, README and guide
- investigate having #[pyclass] and #[pyfunction] directly in the #[pymodule]

Co-authored-by: David Hewitt <[email protected]>
Co-authored-by: Georg Brandl <[email protected]>
Tpt added a commit to Tpt/pyo3 that referenced this pull request Feb 9, 2024
Based on PyO3#2367 and PyO3#3294

Allows to export classes, native classes, functions and submodules and provide an init function

See test/test_module.rs for an example

Future work:
- update examples, README and guide
- investigate having #[pyclass] and #[pyfunction] directly in the #[pymodule]

Co-authored-by: David Hewitt <[email protected]>
Co-authored-by: Georg Brandl <[email protected]>
Tpt added a commit to Tpt/pyo3 that referenced this pull request Feb 9, 2024
Based on PyO3#2367 and PyO3#3294

Allows to export classes, native classes, functions and submodules and provide an init function

See test/test_module.rs for an example

Future work:
- update examples, README and guide
- investigate having #[pyclass] and #[pyfunction] directly in the #[pymodule]

Co-authored-by: David Hewitt <[email protected]>
Co-authored-by: Georg Brandl <[email protected]>
Tpt added a commit to Tpt/pyo3 that referenced this pull request Feb 9, 2024
Based on PyO3#2367 and PyO3#3294

Allows to export classes, native classes, functions and submodules and provide an init function

See test/test_module.rs for an example

Future work:
- update examples, README and guide
- investigate having #[pyclass] and #[pyfunction] directly in the #[pymodule]

Co-authored-by: David Hewitt <[email protected]>
Co-authored-by: Georg Brandl <[email protected]>
Tpt added a commit to Tpt/pyo3 that referenced this pull request Feb 9, 2024
Based on PyO3#2367 and PyO3#3294

Allows to export classes, native classes, functions and submodules and provide an init function

See test/test_module.rs for an example

Future work:
- update examples, README and guide
- investigate having #[pyclass] and #[pyfunction] directly in the #[pymodule]

Co-authored-by: David Hewitt <[email protected]>
Co-authored-by: Georg Brandl <[email protected]>
@davidhewitt
Copy link
Member

Superseded by #3815

Tpt added a commit to Tpt/pyo3 that referenced this pull request Feb 16, 2024
Based on PyO3#2367 and PyO3#3294

Allows to export classes, native classes, functions and submodules and provide an init function

See test/test_module.rs for an example

Future work:
- update examples, README and guide
- investigate having #[pyclass] and #[pyfunction] directly in the #[pymodule]

Co-authored-by: David Hewitt <[email protected]>
Co-authored-by: Georg Brandl <[email protected]>
Tpt added a commit to Tpt/pyo3 that referenced this pull request Feb 19, 2024
Based on PyO3#2367 and PyO3#3294

Allows to export classes, native classes, functions and submodules and provide an init function

See test/test_module.rs for an example

Future work:
- update examples, README and guide
- investigate having #[pyclass] and #[pyfunction] directly in the #[pymodule]

Co-authored-by: David Hewitt <[email protected]>
Co-authored-by: Georg Brandl <[email protected]>
Tpt added a commit to Tpt/pyo3 that referenced this pull request Feb 21, 2024
Based on PyO3#2367 and PyO3#3294

Allows to export classes, native classes, functions and submodules and provide an init function

See test/test_module.rs for an example

Future work:
- update examples, README and guide
- investigate having #[pyclass] and #[pyfunction] directly in the #[pymodule]

Co-authored-by: David Hewitt <[email protected]>
Co-authored-by: Georg Brandl <[email protected]>
Tpt added a commit to Tpt/pyo3 that referenced this pull request Feb 23, 2024
Based on PyO3#2367 and PyO3#3294

Allows to export classes, native classes, functions and submodules and provide an init function

See test/test_module.rs for an example

Future work:
- update examples, README and guide
- investigate having #[pyclass] and #[pyfunction] directly in the #[pymodule]

Co-authored-by: David Hewitt <[email protected]>
Co-authored-by: Georg Brandl <[email protected]>
github-merge-queue bot pushed a commit that referenced this pull request Feb 24, 2024
* #[pymodule] mod some_module { ... } v3

Based on #2367 and #3294

Allows to export classes, native classes, functions and submodules and provide an init function

See test/test_module.rs for an example

Future work:
- update examples, README and guide
- investigate having #[pyclass] and #[pyfunction] directly in the #[pymodule]

Co-authored-by: David Hewitt <[email protected]>
Co-authored-by: Georg Brandl <[email protected]>

* tests: group exported imports

* Consolidate pymodule macro code to avoid duplicates

* Makes pymodule_init take Bound<'_, PyModule>

* Renames #[pyo3] to #[pymodule_export]

* Gates #[pymodule] mod behind the experimental-declarative-modules feature

* Properly fails on functions inside of declarative modules

---------

Co-authored-by: David Hewitt <[email protected]>
Co-authored-by: Georg Brandl <[email protected]>
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.

3 participants