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

release: 0.23.0 #4651

Merged
merged 1 commit into from
Nov 15, 2024
Merged

release: 0.23.0 #4651

merged 1 commit into from
Nov 15, 2024

Conversation

davidhewitt
Copy link
Member

@davidhewitt davidhewitt commented Oct 25, 2024

This is a branch to prep the 0.23 release. I think

  • This release has a lot of stuff in here already which is going to push users to do migration work thanks to the freethreaded support and also IntoPyObject. I am nervous about adding much more.
  • Python 3.13.0 has been live a couple of weeks and it would be great for us to just ship as soon as we have passable freethreading support so that the downstream ecosystem can be unblocked.

I hope that we might be able to get this shipped a week from now, 1st November. My list of what needs to go in:

I think the design scope of those is relatively clear, even if that's still a fair bit of typing to do!

Note that this means that not everything which we listed in #4554 is making it in; I am sorry for the pieces we missed. I'd rather ship this, get feedback and start working on fixes while we also prep those next pieces for 0.24. I'd love it if we can do some smaller, gentler releases for 0.24 and the few after that.

... that said, one of the things not making it into 0.23 will be FromPyObject changes, which I think will make sense to get on with ASAP 👀

@davidhewitt
Copy link
Member Author

With all items done, I have rebased this and curated the (massive) CHANGELOG entry. I've provisionally marked this for release on Saturday (9th Nov).

In the interim, I will try to pass eyes over the docs and improve what I can before release.

@davidhewitt davidhewitt marked this pull request as ready for review November 5, 2024 21:40
@davidhewitt
Copy link
Member Author

I think we are still missing docs for migration particularly of #[pyclass] requiring Sync. I have had a lot of family burden this past week so I've run out of energy to write these myself the past few days.

I might have more energy tomorrow; it is possible that instead of releasing I will push a docs PR, and perhaps as long as there are some docs we can release on Sunday. I think it'd be pretty unfair on users if we released without that doc explaining the Sync changes.

It is also possible that I won't get around to the docs writing tomorrow, family situation will be ongoing for the foreseeable future. Will make a judgement call what to do then.

@davidhewitt
Copy link
Member Author

I didn't have time & energy to make progress yesterday, unclear if today will be any better. I have a train journey tomorrow so I can probably get the docs written at least.

If #4216 (reply in thread) reproduces on main, we should probably fix before release. It'll probably be a simple fix to make the enum code work more like #[pyo3(get)], but will be potentially breaking so not shippable in a post-release patch.

@Icxolu
Copy link
Contributor

Icxolu commented Nov 10, 2024

I attempted a fix in #4694. It makes it work, but we should probably overhaul complex enums in the future again to better reuse our code.

@davidhewitt
Copy link
Member Author

Thanks!

With that fix now merged, and I think at least some basic docs in #4695, I think I will rebase and release this tomorrow evening (12th Nov).

Tomorrow in working hours I will try updating pydantic-core to use this branch, which will be a last-minute smoke test that nothing horribly broken is lying waiting to be released. Assuming that all goes fine, I see no reason to wait any further!

@ngoldbaum
Copy link
Contributor

FWIW I was looking at a few downstream projects today as well. It looks like the rust parts of cryptography and all of rpds-py should be straightforward to update. The other projects are on older PyO3 versions or are using the FFI to define extensions. I haven't yet run into a downstream error about something not being Sync.

@davidhewitt
Copy link
Member Author

Unfortunately the pydantic-core upgrade didn't go very well;

  • hit some recursion errors with type inference trying to evaluate IntoPyObject for &&&&&&&&&&&&&&[...]&&Bound. Not clear to me why yet and if it will be a recurrent problem for users to deal with which might require some changes.
  • my family and I are sick again, so I spent most the day sleeping

I will try to look again ASAP when I have some more energy, others are also welcome to investigate.

@alex
Copy link
Contributor

alex commented Nov 12, 2024 via email

@ngoldbaum
Copy link
Contributor

@ngoldbaum
Copy link
Contributor

I noticed yesterday in rpds.py and today in cryptography that I'm making a lot of changes along the lines of this to migrate from ToPyObject to IntoPyObject:

@@ -60,16 +60,19 @@ fn decode_dss_signature(
 ) -> Result<pyo3::PyObject, CryptographyError> {
     let sig = asn1::parse_single::<DssSignature<'_>>(data)?;

-    Ok((
+    (
         big_byte_slice_to_py_int(py, sig.r.as_bytes())?,
         big_byte_slice_to_py_int(py, sig.s.as_bytes())?,
     )
-        .to_object(py))
+        .into_pyobject(py)
+        .map_err(Into::into)
+        .map(pyo3::BoundObject::into_any)
+        .map(pyo3::BoundObject::unbind)
 }

Maybe the docs and migration guide could point to BoundObject as a useful thing for this sort of operation? Or am I doing something weird here?

@Icxolu
Copy link
Contributor

Icxolu commented Nov 13, 2024

That seems reasonable to me. I believe in this specific case there are no generic involved, so you get a concrete type back (PyResult<Bound<'py, PyTuple>>). So I think you could also write this as

(...).into_pyobject(py)?.into_any().unbind()

here (or if we want to use map, directly referring to Bounds methods is also possible). BoundObject comes more into play it the type you're converting is a generic. Sometimes it can also be useful to propagate the target type up to get stronger typing throughout the code, but that's a decision to be taken on a case by case basis I would say.

@ngoldbaum
Copy link
Contributor

Ah I see how I don't need to use map since I have a Result - there were a few places in rpds.py where it really was a map and BoundObject makes more sense. I'll try to write something up and ping you to review it.

@davidhewitt
Copy link
Member Author

👍 I do think there is going to be some common helpers / trait we can put into PyO3 to make those kinds of patterns easier, but that can be a post-release QoL improvement.

I'm a bit brighter today so I'm trying to continue to push forward with final testing on pydantic-core. I am going to try to find a minimal reproduction for the recursion overflow. In the meanwhile I've also found #4701.

The WIP branch is at pydantic/pydantic-core#1450, if I silence deprecation warnings the upgrade is "easy", there was a couple of Sync fixups to classes. In particular it's ToPyObject -> IntoPyObject which seems to be the only challenging bit at least.

@ngoldbaum
Copy link
Contributor

Should this work?

error[E0277]: can't compare `pyo3::Bound<'_, PyInt>` with `pyo3::Bound<'_, PyInt>`
   --> src/rust/src/backend/dh.rs:475:31
    |
475 |         Ok(self.y.bind(py).eq(other.y.bind(py))?
    |                            -- ^^^^^^^^^^^^^^^^ no implementation for `pyo3::Bound<'_, PyInt> == pyo3::Bound<'_, PyInt>`
    |                            |
    |                            required by a bound introduced by this call
    |
    = help: the trait `PartialEq<pyo3::Bound<'_, PyInt>>` is not implemented for `pyo3::Bound<'_, PyInt>`

I also see similar issues caused by PartialEq not being implemented between PyInt and PyAny.

@Icxolu
Copy link
Contributor

Icxolu commented Nov 13, 2024

I think you want use PyAnyMethods::eq and not PartialEq::eq? Normally PyAnyMethods should work through auto-deref of Bound, but in this case it seems to need an explicit deref (I assume because of the naming collision) (**self.y.bind(py)).eq(other.y.bind(py)

@davidhewitt
Copy link
Member Author

I managed to capture a minimal repro for the bound overflow: #4702

@davidhewitt
Copy link
Member Author

The discussion in #4702 has convinced me the current situation is ok; if users have trouble upgrading we can point them to it, and work on QoL improvements as patch releases / future releases.

Let's proceed to get docs merged and release 👍

@davidhewitt
Copy link
Member Author

Ok, everything merged in, I will merge this into main now, assuming that succeeds I will then release!

@davidhewitt davidhewitt added this pull request to the merge queue Nov 15, 2024
Merged via the queue into main with commit 8e9b497 Nov 15, 2024
46 checks passed
@davidhewitt davidhewitt deleted the release-0.23 branch November 15, 2024 15:17
@davidhewitt
Copy link
Member Author

Plz hold, #4710

@davidhewitt
Copy link
Member Author

https://github.com/PyO3/pyo3/releases/tag/v0.23.0 🎉

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

Successfully merging this pull request may close these issues.

4 participants