Unify #[pyfunction] and #[pyfn]

[davidhewitt]

It seems confusing to me to have two different attributes, #[pyfunction] and #[pyfn], which both wrap Rust functions to create python functions. Pyo3 users need to know which applies in two different contexts.

I'd argue we should merge these. Then there's only one attribute which pyo3 users need to know about. It would also help us to simplify / refactor the macro code if we didn't need two different parsing systems.

I think this would need to go in the following steps:

1. Allow #[pyfunction] to be used in place of #[pyfn]
2. Change the docs to describe #[pyfunction] everywhere
3. Change #[pyfn] to expand to #[pyfunction]
4. (Later on) change #[pyfn] to emit an error, stating the #[pyfunction] invocation to use instead
5. (Much later on) remove #[pyfn]

I've already opened a PR #693 to achieve step 1.

[kngwyu]

But... are they so confusing?
I can see the difference clearly from the guide.

Or am I too accustomed?

[davidhewitt]

The guide is clear enough on the differences. It's only a little confusing - but mostly I think for new users of pyo3.

Another downside to having two different attributes is that they might drift apart over time if we're not careful. For example, if #692 lands, then the way that each of these specify the python name is different:

#[pyfn(m, "foo")]
fn bar() { }

vs

#[pyfunction]
#[name = "foo"]
fn bar() { }

It seems like a good idea to me to simplify the codebase by unifying these attributes.

[pganssle]

@kngwyu I would not say they are confusing if you read the guide, but I agree with @davidhewitt - these two procedural macros apparently do the same thing but in two different contexts.

Consider that even if the documentation clarifies this, it's definitely going to be hard to remember which one to use in which context unless you do it basically every day, so you'll have to look it up every time. Readers of the code who aren't necessarily reading the documentation will wonder why one or the other is used, etc.

If it's possible to unify them into a single macro, IMO, it should definitely be done.

[kngwyu]

I'd rather introduce

#[pymodule] 
mod mymod {
    #[pyfunction] 
    fn myfunction(py: Python, ...) {}
}

and make pyfn soft-duplicated.

[davidhewitt]

I do like this new syntax for #[pymodule] very much, and would be happy to go down that road. How would you propose classes and attributes to be added to a #[pymodule] in this design? I'm thinking something like this:

#[pymodule]
mod mymod {
    #[pyfunction]
    fn myfunction(py: Python, ...) { ... }

    #[pyclass]
    struct MyClass { ... }

    #[pymethods]
    impl MyClass { ... } 

    #[pyattribute]
    static myattr: i32 = 5;
}

[programmerjake]

I do like this new syntax for #[pymodule] very much, and would be happy to go down that road. How would you propose classes and attributes to be added to a #[pymodule] in this design? I'm thinking something like this:

<omitted>

I think attributes should be const rather than static since the value accessible from Rust is not the same variable as the value accessible from Python. If non-const attributes are needed, they can be added in the module #[init] function:

#[pymodule]
mod my_mod {
    #[pyclass]
    struct MyClass { ... }

    #[pyfunction]
    fn myfunction(py: Python, ...) { ... }

    #[pyattribute]
    // can override name on all module members using #[name]
    #[name = "MY_PYTHON_NAME"]
    pub const MY_CONST: &str = "My str";

    #[init]
    fn my_init(py: Python, module: &PyModule) {
        // custom init code; called after adding all other module members
        // can add non-const attributes here using module.add()
    }

    // add in external #[pyclass], #[pyfunction]
    #[pyclass]
    #[name = "NameStillWorksHere"]
    pub use path::to::external::MyClass;
    #[pyfunction]
    pub use path::to::external::my_function;
}

[davidhewitt]

I think attributes should be const rather than static

I did consider const, but wondered if that might be misleading because nothing in Python is ever really const 😄 . I would be happy to go with it if that's the consensus.

I really like your idea of pub use to add extra items to the module.

[kngwyu]

I'm sorry I noticed #[proc_macro] mod m { ... } is unstable today(rust-lang/rust#54727).

@programmerjake

I experimentally implemented a similar syntax, but had some problems:

• use function; raises not used warning
• pub use function; requires function is pub

[programmerjake]

I'm sorry I noticed #[proc_macro] mod m { ... } is unstable today(rust-lang/rust#54727).

It's stable (in the latest releases): rust-lang/rust#64273

What's unstable is non-inline mod:

#[my_proc_macro]
mod my_external_mod;

It's unstable because they haven't yet decided if the proc-macro will see mod my_external_mod; or mod my_external_mod { ... }.

@programmerjake

I experimentally implemented a similar syntax, but had some problems:

• use function; raises not used warning

just use the name that the use statement introduces. alternatively decorate it with the appropriate #[allow(...)].

• pub use function; requires function is pub

pub or pub(<path>) should be used only when the user specifies it, it should have no effect on the generated Python bindings, all it should do is make the Rust function visible to other Rust code. What makes the function visible to Python is the #[pyfunction] annotation, which should be independent of Rust visibility.

[kngwyu]

@programmerjake

It's stable (in the latest releases): rust-lang/rust#64273

Wow, thank you for letting me know that 👍

just use the name that the use statement introduces

Another idea: How about removing the whole use statement?

[programmerjake]

just use the name that the use statement introduces

Another idea: How about removing the whole use statement?

I would leave the use statement in there since the imported function could be used from the user's Rust code:

#[pymodule]
mod my_mod {
    #[pyfunction]
    use my::other_fn;

    #[pyfunction]
    fn my_fn() -> i32 {
        other_fn(123)
    }
}

[birkenfeld]

Proc-macros on mod are stable in 1.48.0, so we could make a pass at implementing a new pymodule now.

[davidhewitt]

Indeed, I'm dangerously tempted to try to do this very soon! If anyone else wants to play with it, feel free to claim this and have a go and I'll be a happy reviewer / co-designer 😄