python-trio / trio

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

add deprecation warning for specifying strict_exception_groups=False #2941

Closed jakkdl closed 5 months ago

jakkdl commented 5 months ago

Wasn't totally sure which issue to refer to, ended up going with #2929

the removed with block in test_subprocess was added to try and track down some weirdness in https://github.com/python-trio/trio/pull/2886#discussion_r1449035885 - but I don't think it served much of a purpose in the end, so opted now to remove it instead of wrapping it in a pytest.warns.

codecov[bot] commented 5 months ago

Codecov Report

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

Comparison is base (b6a31d7) 99.64% compared to head (06168d2) 99.64%. Report is 1 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #2941 +/- ## ======================================= Coverage 99.64% 99.64% ======================================= Files 116 117 +1 Lines 17521 17564 +43 Branches 3151 3166 +15 ======================================= + Hits 17459 17502 +43 Misses 43 43 Partials 19 19 ``` | [Files](https://app.codecov.io/gh/python-trio/trio/pull/2941?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/\_run.py](https://app.codecov.io/gh/python-trio/trio/pull/2941?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-trio#diff-c3JjL3RyaW8vX2NvcmUvX3J1bi5weQ==) | `99.66% <100.00%> (+<0.01%)` | :arrow_up: | | [src/trio/\_core/\_tests/test\_run.py](https://app.codecov.io/gh/python-trio/trio/pull/2941?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-trio#diff-c3JjL3RyaW8vX2NvcmUvX3Rlc3RzL3Rlc3RfcnVuLnB5) | `100.00% <100.00%> (ø)` | | | [...ts/test\_deprecate\_strict\_exception\_groups\_false.py](https://app.codecov.io/gh/python-trio/trio/pull/2941?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-trio#diff-c3JjL3RyaW8vX3Rlc3RzL3Rlc3RfZGVwcmVjYXRlX3N0cmljdF9leGNlcHRpb25fZ3JvdXBzX2ZhbHNlLnB5) | `100.00% <100.00%> (ø)` | | | [src/trio/\_tests/test\_subprocess.py](https://app.codecov.io/gh/python-trio/trio/pull/2941?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-trio#diff-c3JjL3RyaW8vX3Rlc3RzL3Rlc3Rfc3VicHJvY2Vzcy5weQ==) | `99.23% <ø> (-0.01%)` | :arrow_down: |
CoolCat467 commented 5 months ago

Making sure, nothing can go wrong with the literal is False compare? No one could set strict exception groups to a value that could be interpreted as the False branch?

jakkdl commented 5 months ago

Making sure, nothing can go wrong with the literal is False compare? No one could set strict exception groups to a value that could be interpreted as the False branch?

Right, if somebody is supplying a falsy value of an incorrect type they're not gonna get a warning. So I suppose I could do

if not strict_exception_groups and strict_exception_groups is not None:
    ...

Don't think we need to be that defensive, but I guess it also doesn't hurt.