Skip to content
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions newsfragments/5375.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed `PyModuleMethods::add_submodul()` to use the last segment of the submodule name as the attribute name on the parent module instead of using the full name.
6 changes: 4 additions & 2 deletions pyo3-macros-backend/src/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,7 @@ pub fn pymodule_module_impl(
let gil_used = options.gil_used.is_some_and(|op| op.value.value);

let initialization = module_initialization(
&full_name,
&name,
ctx,
quote! { __pyo3_pymodule },
Expand Down Expand Up @@ -451,6 +452,7 @@ pub fn pymodule_function_impl(
let gil_used = options.gil_used.is_some_and(|op| op.value.value);

let initialization = module_initialization(
&name.to_string(),
&name,
ctx,
quote! { ModuleExec::__pyo3_module_exec },
Expand Down Expand Up @@ -499,6 +501,7 @@ pub fn pymodule_function_impl(
}

fn module_initialization(
full_name: &str,
name: &syn::Ident,
ctx: &Ctx,
module_exec: TokenStream,
Expand All @@ -508,8 +511,7 @@ fn module_initialization(
) -> TokenStream {
let Ctx { pyo3_path, .. } = ctx;
let pyinit_symbol = format!("PyInit_{name}");
let name = name.to_string();
let pyo3_name = LitCStr::new(&CString::new(name).unwrap(), Span::call_site());
let pyo3_name = LitCStr::new(&CString::new(full_name).unwrap(), Span::call_site());

let mut result = quote! {
#[doc(hidden)]
Expand Down
9 changes: 7 additions & 2 deletions src/types/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ pub struct PyModule(PyAny);
pyobject_native_type_core!(PyModule, pyobject_native_static_type_object!(ffi::PyModule_Type), #checkfunction=ffi::PyModule_Check);

impl PyModule {
/// Creates a new module object with the `__name__` attribute set to `name`.
/// Creates a new module object with the `__name__` attribute set to `name`. When creating
/// a submodule pass the full path as the name such as `top_level.name`.
///
/// # Examples
///
Expand Down Expand Up @@ -513,7 +514,11 @@ impl<'py> PyModuleMethods<'py> for Bound<'py, PyModule> {
}

fn add_submodule(&self, module: &Bound<'_, PyModule>) -> PyResult<()> {
let name = module.name()?;
let name = module
.name()?
.call_method1("rpartition", (".",))?
.get_item(2)?
.cast_into::<PyString>()?;
Copy link
Member

Choose a reason for hiding this comment

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

Probably marginally more efficient to go through Rust and fewer error branches:

let name = module.name()?;
let name = name.to_cow()?;
let name = name.rsplit_once(b'.').map_or(&name, |(_, last_part)| last_part);

self.add(name, module)
}

Expand Down
21 changes: 20 additions & 1 deletion tests/test_declarative_module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ fn double_value(v: &ValueClass) -> usize {
v.value * 2
}

#[pymodule]
#[pymodule(module = "declarative_module")]
mod declarative_submodule {
#[pymodule_export]
use super::{double, double_value};
Expand Down Expand Up @@ -204,6 +204,25 @@ fn test_declarative_module() {
py_assert!(py, m, "not hasattr(m, 'BAR')");
py_assert!(py, m, "m.type == '!'");
py_assert!(py, m, "not hasattr(m, 'NOT_EXPORTED')");

// submodule dunder name and attribute name
// declarative_module.inner is declared inside
py_assert!(py, m, "m.inner.__name__ == 'declarative_module.inner'");
py_assert!(py, m, "'inner' in m.__dict__");
py_assert!(py, m, "'declarative_module.inner' not in m.__dict__");

// since declarative_submodule is declared outside, but the parent module name is passed
py_assert!(
py,
m,
"m.declarative_submodule.__name__ == 'declarative_module.declarative_submodule'"
);
py_assert!(py, m, "'declarative_submodule' in m.__dict__");
py_assert!(
py,
m,
"'declarative_module.declarative_submodule' not in m.__dict__"
);
})
}

Expand Down
34 changes: 32 additions & 2 deletions tests/test_module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -365,10 +365,10 @@ fn superfunction() -> String {
#[pymodule]
fn supermodule(module: &Bound<'_, PyModule>) -> PyResult<()> {
module.add_function(wrap_pyfunction!(superfunction, module)?)?;
let module_to_add = PyModule::new(module.py(), "submodule")?;
let module_to_add = PyModule::new(module.py(), "supermodule.submodule")?;
submodule(&module_to_add)?;
module.add_submodule(&module_to_add)?;
let module_to_add = PyModule::new(module.py(), "submodule_with_init_fn")?;
let module_to_add = PyModule::new(module.py(), "supermodule.submodule_with_init_fn")?;
submodule_with_init_fn(&module_to_add)?;
module.add_submodule(&module_to_add)?;
Ok(())
Expand Down Expand Up @@ -396,6 +396,36 @@ fn test_module_nesting() {
supermodule,
"supermodule.submodule_with_init_fn.subfunction() == 'Subfunction'"
);

// submodule dunder name and attribute name
py_assert!(
py,
supermodule,
"supermodule.submodule.__name__ == 'supermodule.submodule'"
);
py_assert!(py, supermodule, "'submodule' in supermodule.__dict__");
py_assert!(
py,
supermodule,
"'supermodule.submodule' not in supermodule.__dict__"
);

// submodule_with_init_fn dunder name and attribute name
py_assert!(
py,
supermodule,
"supermodule.submodule_with_init_fn.__name__ == 'supermodule.submodule_with_init_fn'"
);
py_assert!(
py,
supermodule,
"'submodule_with_init_fn' in supermodule.__dict__"
);
py_assert!(
py,
supermodule,
"'supermodule.submodule_with_init_fn' not in supermodule.__dict__"
);
});
}

Expand Down
Loading