python-trio / trio

Trio – a friendly Python library for async concurrency and I/O
https://trio.readthedocs.io
Other
6.22k stars 344 forks source link

add __copy__ and __deep_copy__ to Cancelled #3105

Closed jakkdl closed 15 hours ago

jakkdl commented 1 month ago

See https://github.com/python-trio/flake8-async/issues/298 for why I want to be able to copy Cancelled.

I suppose there's a minor footgun added if people start copying Cancelled, NoPublicConstructor was added for a reason, but I think the gain outweighs it. Though of course another option is to simply get rid of NoPublicConstructor

I initially tried to write cleaner methods, where I was using __dict__ and the like, but they do not appear to pick up the relevant dunders so I simply listed everything relevant from https://docs.python.org/3/library/exceptions.html#exception-context

codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 99.58%. Comparing base (2a66a0d) to head (9aa6d36). Report is 159 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #3105 +/- ## ======================================= Coverage 99.58% 99.58% ======================================= Files 121 121 Lines 18166 18191 +25 Branches 3275 3278 +3 ======================================= + Hits 18091 18116 +25 Misses 52 52 Partials 23 23 ``` | [Files with missing lines](https://app.codecov.io/gh/python-trio/trio/pull/3105?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-trio) | Coverage Δ | | |---|---|---| | [src/trio/\_core/\_exceptions.py](https://app.codecov.io/gh/python-trio/trio/pull/3105?src=pr&el=tree&filepath=src%2Ftrio%2F_core%2F_exceptions.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-trio#diff-c3JjL3RyaW8vX2NvcmUvX2V4Y2VwdGlvbnMucHk=) | `100.00% <100.00%> (ø)` | | | [src/trio/\_core/\_tests/test\_run.py](https://app.codecov.io/gh/python-trio/trio/pull/3105?src=pr&el=tree&filepath=src%2Ftrio%2F_core%2F_tests%2Ftest_run.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-trio#diff-c3JjL3RyaW8vX2NvcmUvX3Rlc3RzL3Rlc3RfcnVuLnB5) | `100.00% <100.00%> (ø)` | |

🚨 Try these New Features:

jakkdl commented 1 month ago

This is a far better copy than most exception objects have, FYI. (I left a comment on the flake8-trio issue that motivated this.)

Thank you! I think we don't need to bother doing it either then, and I'll throw out the complicated logic.

jakkdl commented 1 month ago

it appears copy.copy has bigger issues with exceptions than trio.Cancelled, see https://github.com/python-trio/flake8-async/issues/298#issuecomment-2424971988 I'll probably close this PR

jakkdl commented 3 weeks ago

okay I think my original reasoning for this is no longer valid - copying is too unreliable to use in a general library like trio-websocket that needs to handle arbitrary user exceptions, but we should probably make it possible to copy & pickle Cancelled for general usability reasons.

That's fairly straightforward by implementing __reduce__, but it's maybe bad that all other NoPublicConstructor classes remain unpickleable? I'm at least finding one somewhat related issue: https://github.com/python-trio/trio/issues/2135