python-trio / trio-websocket

WebSocket client and server implementation for Python Trio
MIT License
70 stars 26 forks source link

Use built-in exception groups in Python 3.11+ #180

Closed mgorny closed 1 year ago

mgorny commented 1 year ago

Python 3.12+ features built-in exception group support, and does not require third-party exceptiongroup package. Make the import and the dependency conditional to older Python versions.

belm0 commented 1 year ago

Isn't ExceptionGroup in Python >= 3.11, not 3.12?

Practically, how important is this change? Because the exceptiongroup shim package is a no-op under Python >= 3.11.

If this package is imported on Python 3.11 or later, the built-in implementations of the exception group classes are used instead, TracebackException is not monkey patched and the exception hook won’t be installed.

mgorny commented 1 year ago

Isn't ExceptionGroup in Python >= 3.11, not 3.12?

Yeah, you're right, sorry.

Practically, how important is this change? Because the exceptiongroup shim package is a no-op under Python >= 3.11.

It's important for us since we longer package exceptiongroup in Gentoo for Python 3.12 (because its test suite no longer works), so it's blocking trio-websocket for us.

mgorny commented 1 year ago

I've updated the PR to use 3.11.

belm0 commented 1 year ago

we [no] longer package exceptiongroup in Gentoo for Python 3.12 (because its test suite no longer works)

Woludn't it be better to fix this root problem of the exceptiongroup package, rather than updating many packages like trio-websocket to conditionally include exceptiongroup?

mgorny commented 1 year ago

Woludn't it be better to fix this root problem of the exceptiongroup package, rather than updating many packages like trio-websocket to conditionally include exceptiongroup?

No, it wouldn't solve the problem. exceptiongroup is a backport package (it says that right on its pypi page) that is meaningless for new Python versions, and you are not supposed to import it unconditionally like that. Gentoo has already switched to Python 3.11 by default, and Python 3.10 is due to be removed soon, and our exceptiongroup package will be removed along with it.

belm0 commented 1 year ago

exceptiongroup is a backport package (it says that right on its pypi page) that is meaningless for new Python versions, and you are not supposed to import it unconditionally like that.

On the contrary, exceptiongroup, like many backport packages, appears designed to be used unconditionally, with the package internally handling the case where it isn't needed. This makes things easier for both the library package directly using the backport (which may try to support a wide range of Python versions) and any packages depending on that library:

I don't expect to change your mind, but I think Gentoo is being too aggressive about purging backport packages, and it would be better to fix the exceptiongroup test suite issue.

thesamesam commented 1 year ago

The problem is, with experience, we've seen these backport packages tend to rot and people lose interest in keeping them working. At some point, they have to stop being used anyway, but the point of rot (like this case w/ the test suite) tends to happen a while before packages drop it entirely because their range of supported Python versions change. But I take the point wrt requirements.

mgorny commented 1 year ago

conditional imports dependencies become a headache when working with pinned requirements, because a separate requirements file is needed for every combination of conditional dependency

I disagree. They cause no more pain than some (possibly transitive) dependencies having narrower Python version range support than your package. In that case, older versions need to be pinned for older Python versions, and you're effectively stuck with multiple dependency files anyway.

That said, as far as distribution packagers are concerned, you can pin dependencies including exceptiongroup. We're not going to use the pins anyway.

belm0 commented 1 year ago

They cause no more pain than some (possibly transitive) dependencies having narrower Python version range support than your package ... That said, as far as distribution packagers are concerned, you can pin dependencies including exceptiongroup. We're not going to use the pins anyway.

A common use of pinned requirements is for reproducible CI builds. If there's a conditional dependency, it's important to exercise that. So instead of a single pinned requirements file that could be passed to every Python version in the CI matrix, a project with say two conditional dependencies (one on Python 3.7, one on Python 3.11) now needs 3 requirements files, and must use the correct one for each Python version in the CI matrix. And since conditional depends are transitive, any parent packages face the same burden.

mgorny commented 1 year ago

This is already the case if any of your (transitive) dependencies have optional dependencies themselves. Even if you managed to cover every possible valid combination of dependencies, this doesn't cover all possible scenarios. Even a totally unrelated package being installed in the environment can possibly break stuff (and we've witnessed that more than once).

belm0 commented 1 year ago

merged equivalent via #183