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

py_any.downcast::<PyIterator>() behaviour is different on windows vs macos and linux #2913

Closed
samuelcolvin opened this issue Jan 25, 2023 · 3 comments · Fixed by #2914
Closed
Labels

Comments

@samuelcolvin
Copy link
Contributor

Bug Description

This is an extension of #2902

On Linux and MacOS, py_any.downcast::<PyIterator>() is only Ok for generators.

On windows it appears to be Ok for strings and anything which is a sequence.

Steps to Reproduce

Should be as simple as

let s = "foobar".to_object().into_ref(py);
let iter: PyIterator = s.downcast()?;

Should fail on linux and macos, but pass on windows.

Backtrace

No response

Your operating system and version

Ubuntu

Your Python version (python --version)

3.10

Your Rust version (rustc --version)

cargo 1.68.0-nightly (2381cbdb4 2022-12-23)

Your PyO3 version

v0.18

How did you install python? Did you use a virtualenv?

pyenv

Additional Info

No response

@davidhewitt
Copy link
Member

So I've just run the following test locally on PyO3 main, both Windows and Unix, Python 3.10 and 3.11:

#[test]
fn test_iterator_downcast() {
    Python::with_gil(|py| {
        let s = "foobar".to_object(py).into_ref(py);
        let iter: &pyo3::types::PyIterator = s.downcast().unwrap();
    })
}

In all cases, I get the expected failure:

---- test_iterator_downcast stdout ----
thread 'test_iterator_downcast' panicked at 'called `Result::unwrap()` on an `Err` value: PyDowncastError { from: 'foobar', to: "Iterator" }', tests/test_downcast.rs:7:59
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

So I don't think this is a bug in PyO3. Instead let's go back to the discussion and get some more information about how you're setting up your test environment, so we can understand what's different about your Windows setup.

@davidhewitt davidhewitt closed this as not planned Won't fix, can't repro, duplicate, stale Jan 25, 2023
@davidhewitt
Copy link
Member

Actually, with your example Python class in #2902 I can repro this, and will push a fix shortly.

@samuelcolvin
Copy link
Contributor Author

My mistake, it was a subclass of str, sorry created this in a hurry.

bors bot added a commit that referenced this issue Jan 28, 2023
2914: correct ffi definition of PyIter_Check r=davidhewitt a=davidhewitt

Closes #2913 

It looks like what is happening is that PyO3 was relying on an outdated macro form of `PyIter_Check` which is now a CPython implementation detail, which would explain why it was behaving inconsistently on different platforms (likely due to differences in linkers / implementations).

The test I've pushed succeeds, but fails to compile due to a hygiene bug! I'm done for tonight so I'll take a look at that soon and then rebase this after.

Co-authored-by: David Hewitt <1939362+davidhewitt@users.noreply.github.com>
bors bot added a commit that referenced this issue Jan 29, 2023
2914: correct ffi definition of PyIter_Check r=davidhewitt a=davidhewitt

Closes #2913 

It looks like what is happening is that PyO3 was relying on an outdated macro form of `PyIter_Check` which is now a CPython implementation detail, which would explain why it was behaving inconsistently on different platforms (likely due to differences in linkers / implementations).

The test I've pushed succeeds, but fails to compile due to a hygiene bug! I'm done for tonight so I'll take a look at that soon and then rebase this after.

Co-authored-by: David Hewitt <1939362+davidhewitt@users.noreply.github.com>
samuelcolvin added a commit to pydantic/pydantic that referenced this issue Feb 3, 2023
samuelcolvin added a commit to pydantic/pydantic that referenced this issue Feb 3, 2023
* adopting pydantic-core serialisation

* fixing more serialization tests

* full support for pydantic exclude __all__

* fix JSON tests

* fix JSON tests

* adding serializer decorator

* rename __pydantic_validation_schema__ -> __pydantic_core_schema__

* deque serializer, more tests

* uprev pydantic-core

* fix ci caching

* fix reqs

* fix ci caching

* fix failing tests

* fix mypy tests

* xfail windows tests effected by PyO3/pyo3#2913
@bors bors bot closed this as completed in 806eed5 Feb 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants