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

PyType.is_subclass does not work with two dynamic PyTypes #1984

Closed
Eric-Arellano opened this issue Nov 11, 2021 · 4 comments · Fixed by #1985
Closed

PyType.is_subclass does not work with two dynamic PyTypes #1984

Eric-Arellano opened this issue Nov 11, 2021 · 4 comments · Fixed by #1985

Comments

@Eric-Arellano
Copy link
Contributor

Hello! During Pants's migration to PyO3, I'm stuck on how to get &PyType.is_subclass() working.

This is what I would like to do:

#[pyfunction]
fn check_subclass(typ1: &PyType, typ2: &PyType) -> PyResult<bool> {
  typ1.is_subclass(typ2)
}

If I understand correctly, this is not possible because the API is is_subclass<T>(&self), and it doesn't seem possible to say this due to how Generics work in Rust:

typ1.is_subclass::<typ2>()

For now, I'm thinking I can work around this by writing our own unsafe implementation, but I'd love to be able to contribute a more flexible API to PyO3. I'm thinking is_subclass(&self, T: ToPyObject) -> bool.

What do you think? How would we handle deprecations? (Note that rust-cpython calls it is_subtype_of, maybe that's an option)

Also I'm curious what you think about it returning bool instead of PyResult<bool>?

@birkenfeld
Copy link
Member

Interesting! The current API is fine for some cases, but for cases such as yours we should also offer one that takes the type as parameter. Same for is_instance on PyAny.

For the moment, the best you can do is to call typ1.__subclasscheck__(typ2) using call.

@Eric-Arellano
Copy link
Contributor Author

Thank you for that workaround! Much better than unsafe code.

@birkenfeld
Copy link
Member

Ok, I've started a PR at #1985 to fix this for the next version.

@birkenfeld
Copy link
Member

birkenfeld commented Nov 11, 2021

Also I'm curious what you think about it returning bool instead of PyResult<bool>?

Since the subclass/instance checks can run arbitrary Python code (by overriding __subclasscheck__/__instancecheck__ in the metaclass), we kind of have to keep the possibility for exceptions open.

If you're fine with swallowing exceptions, you can just use unwrap_or(false).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants