Declarative modules next steps

Tpt commented 8 months ago

declarative modules with the #[pymodule] mod X {} syntax is now merged behind the experimental-declarative-modules syntax (#3815).

The next steps are:

Before v0.21:

[x] document the new feature in the guide
[x] allow to export #[pyclass], #[pyfunction] and #[pymodule] that are inlined in the modules (in progress in #3902)

Before v0.22:

[ ] properly register submodules in sys.modules automatically i.e. fix #759 by default
[ ] allow #[pymodule_init] function to return () or PyResult<()>
[x] adding a class to a module via the PyAddToModule trait is currently relying on PyTypeInfo::type_object_bound. This function panics when module initialization fail. It would be better to throw an exception instead like in PyModuleMethods::add_class (in progress in #3919)
[ ] investigate relationship between this and "two-phase initialization" e.g. #2245. In particular we should look at #[pymodule_data] as a way to create a per-module data mechanism (might be postpone to later)
[ ] allow #[pymodule_export] const FOO = ...; to add constants to the modules
[ ] investigate automatically filling the module parameter of #[pyclass] usages inside of decarative modules. #4213
[ ] remove the experimental-declarative-modules feature flag to allow using declarative modules by default
[ ] update examples, README and guide

Before v0.23 (or later?):

[ ] Deprecate #[pyfn]
[ ] Deprecate "function-based" modules

davidhewitt commented 8 months ago

Fantastic, thanks for creating this list! 👍

davidhewitt commented 8 months ago

Possibly worth us remembering the discussion in https://github.com/PyO3/pyo3/pull/3919#discussion_r1515103061 to decide what is allowed to export before we declare stable and remove the feature gate.

nickdrozd commented 8 months ago

Some user feedback: I just upgraded to 0.21.0-beta.0, and the declarative module feature is very nice. It allows for getting rid of a bunch of worthless junk like m.add_function(wrap_pyfunction!(some_func, m)?)?;. Maybe I hate repetitive boilerplate more than most, but this was a big relief for me.

Of course you have all had a lot of discussion about this, and I don't understand most of the details. But from my perspective, the best course would be to bless declarative modules as the standard way of doing things and throw all that old m.add_function stuff in the garbage ASAP.

It's not just that it's a nice feature. It's also that it might end up being on the migration path for a lot of users anyway. Moving from 20 to 21 raised several compiler warnings, and the only way I could figure out to silence all of them was to use a declarative module.

davidhewitt commented 8 months ago

Hey @nickdrozd, thanks for the feedback! I'm pleased you like these, I agree that imperative modules should be deprecated and replaced with these modules as soon as possible. This feature is probably the thing I've wanted most for PyO3 for the longest, except for the Bound API we're finally shipping in 0.21.

We could just call these stable in 0.21, but there are a couple of corner cases of unresolved design that I think could break people so I'd like to get these at least understood before we tell people it's stable. From the guide that we'll ship in 0.21:

Some changes are planned to this feature before stabilization, like automatically filling submodules into sys.modules to allow easier imports (see issue #759) and filling the module argument of inlined #[pyclass] automatically with the proper module name. Macro names might also change.

davidhewitt commented 8 months ago

Just to note against the alternative of 0.21 delaying until these design points are ready: so much is changing in 0.21 already that I'm quite keen to see the release finalised so that users can handle these. There's a lot of stuff beginning to pile up that I've asked to wait until 0.22 already, it would be good to unblock the development flow.

If it turns out that lots of users need to update to declarative modules to solve compile issues, then I think probably we got something wrong with the design of the Bound API migration 😢. Hopefully the additional deprecation warnings we're shipping in 0.21 final compared to 0.21 beta will make the difference.

nickdrozd commented 7 months ago

If it turns out that lots of users need to update to declarative modules to solve compile issues, then I think probably we got something wrong with the design of the Bound API migration 😢. Hopefully the additional deprecation warnings we're shipping in 0.21 final compared to 0.21 beta will make the difference.

I had a setup like this:

#[pymodule]
fn rust_stuff(py: Python, m: &PyModule) -> PyResult<()> {
    // ...
}

After switching to 0.21.0-beta.0, I got this compiler warning:

warning: use of deprecated method `pyo3::methods::Extractor::<T>::extract_gil_ref`: use `&Bound<'_, T>` instead for this function argument
  --> src/lib.rs:30:27
   |
30 | fn rust_stuff(py: Python, m: &PyModule) -> PyResult<()> {
   |                           ^^^^^^^^^^^^
   |
   = note: `#[warn(deprecated)]` on by default

I don't know anything about Bound or extract_gil_ref, so the warning left me baffled. It seemed easier just to switch to the declarative style.

(I am using PyO3 to rewrite a Python project in Rust using a bottom-up leaf-first approach. So Python calls into Rust, but the Rust code never interacts with Python except through pyfunction, pyclass, etc).

davidhewitt commented 7 months ago

Do you think it would have helped if these deprecation messages link you to the PyO3 migration guide?

nickdrozd commented 7 months ago

Do you think it would have helped if these deprecation messages link you to the PyO3 migration guide?

I check the migration guide, but it doesn't say anything about how to update m: &PyModule.

alecandido commented 7 months ago

~~I'm a bit puzzled by the submodule. In the example on the docs is left empty~~

~~but as soon as I try to add something, it fails:~~

#[pymodule]
mod submodule {
    #[pyfunction]    // ■■ consider importing one of these items: `use crate::pyfunction;    `, `use pyo3::pyfunction;    `
    pub fn triple(x: usize) -> usize {    // ■ failed to resolve: function `triple` is not a crate or module  function `triple`
        x * 3
    }
}

error: cannot find attribute `pyfunction` in this scope
  --> src/lib.rs:24:11
   |
24 |         #[pyfunction]
   |           ^^^^^^^^^^
   |
help: consider importing one of these items
   |
24 +         use crate::pyfunction;
   |
24 +         use pyo3::pyfunction;
   |

error[E0433]: failed to resolve: function `triple` is not a crate or module
  --> src/lib.rs:25:16
   |
25 |         pub fn triple(x: usize) -> usize {
   |                ^^^^^^ function `triple` is not a crate or module

~~and without the #[pyfunction] it simply doesn't do anything... (i.e. it's not accessible from the Python module.submodule.triple)~~

~~How is it supposed to be used?~~

Ok, nothing, this was at most a documentation issue, I was just missing a use super::* or use pyo3::prelude::* inside the submodule.

This works perfectly fine:

#[pymodule]
mod submodule {
    use pyo3::prelude::*;

    #[pyfunction]
    pub fn triple(x: usize) -> usize {
        x * 3
    }
}

alecandido commented 7 months ago

Moreover, something weird happens with classes. This is working as expected:

use pyo3::prelude::*;

mod othermod {
    use pyo3::prelude::*;

    #[pyfunction]
    pub fn triple(x: usize) -> usize {
        x * 3
    }
}

#[pymodule]
mod myext {
    #[pymodule_export]
    use super::othermod::triple;
}

but this is not:

use pyo3::prelude::*;

mod othermod {
    use pyo3::prelude::*;

    #[pyclass]    // ■ private associated constant defined here
    pub struct Unit;
}

#[pymodule]    // ■■ associated constant `_PYO3_DEF` is private  private associated constant
mod myext {               
    #[pymodule_export]
    use super::othermod::Unit;
}

error[E0624]: associated constant `_PYO3_DEF` is private
  --> src/lib.rs:10:1
   |
6  |     #[pyclass]
   |     ---------- private associated constant defined here
...
10 | #[pymodule]
   | ^^^^^^^^^^^ private associated constant
   |
   = note: this error originates in the attribute macro `pymodule` (in Nightly builds, run with -Z macro-backtrace for more info)

LilyFoote commented 7 months ago

@alecandido I think this bug is #4036.

alecandido commented 7 months ago

Oh, thanks. I actually searched for _PYO3_DEF in the repo, but forgetting the leading _ (and it didn't show the issue...)

https://github.com/search?q=repo%3APyO3%2Fpyo3+PYO3_DEF&type=issues https://github.com/search?q=repo%3APyO3%2Fpyo3+_PYO3_DEF&type=issues

Sorry...

wyfo commented 7 months ago

Just a remark: exception created with create_exception! are not mentioned in this issue. The only solution I see for the moment is to declare exception outside the module and use #[pymodule_export]. I don't have any idea on how we could improve ergonomics for now.

Tpt commented 7 months ago

Just a remark: exception created with create_exception! are not mentioned in this issue. The only solution I see for the moment is to declare exception outside the module and use #[pymodule_export].

With the current code, yes! I would tend to think the long term solution is #295 but it seems quite a bit of work. An other slighty hacky approach might be to make #[pymodule] macro automatically expose all create_exception! calls inside of the module.

Stargateur commented 5 months ago

Documentation on the module are ignored:

#[pymodule]
/// This is ignored
pub mod foo {
}

Tpt commented 5 months ago

@Stargateur Thank you for reporting this! It is weird, we have a test covering exactly this. May you share a bit more how you load the module and try to extract the documentation?

Stargateur commented 5 months ago

my MCE is as follow:

//! Foo
#![warn(missing_docs)]
use pyo3::pymodule;

#[pymodule]
/// This doesn't remove rustc warning about missing docs
pub mod foo {
}

[package]
name = "foo"
version = "0.0.0"
edition = "2021"

[dependencies.pyo3]
version = "0.21.2"
features = [
    "experimental-declarative-modules",
]

warning: missing documentation for a module
 --> src\lib.rs:6:1
  |
6 | #[pymodule]
  | ^^^^^^^^^^^
  |
note: the lint level is defined here
 --> src\lib.rs:2:9
  |
2 | #![warn(missing_docs)]
  |         ^^^^^^^^^^^^
  = note: this warning originates in the attribute macro `pymodule` (in Nightly builds, run with -Z macro-backtrace for more info)

warning: `foo` (lib) generated 1 warning
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.11s

And cargo doc doesn't show the documentation of module foo either. I think the test you show is about python doc not rust doc.

Tpt commented 5 months ago

@Stargateur Thank you! So, I guess the #[pymodule] macro is not properly reemiting the doc comment on the module. I am going to investigate that.

PyO3 / pyo3

Declarative modules next steps #3900