search

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

Enable ruff's `flake8-trio` rule #2947

Closed CoolCat467 closed 4 months ago

CoolCat467 commented 5 months ago 
          Probably should enable the TRIO rules? Maybe in a separate PR.

Originally posted by @TeamSpen210 in https://github.com/python-trio/trio/issues/2946#issuecomment-1928600531

This pull request enables the flake8-trio ruff rule (but in ruff's system it's marked as TRIO).

codecov[bot] commented 5 months ago

Codecov Report

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

Comparison is base (70c8b76) 99.44% compared to head (9d0008f) 99.64%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #2947 +/- ## ========================================== + Coverage 99.44% 99.64% +0.19% ========================================== Files 117 117 Lines 17564 17565 +1 Branches 3166 3166 ========================================== + Hits 17467 17503 +36 + Misses 79 43 -36 - Partials 18 19 +1 ``` | [Files](https://app.codecov.io/gh/python-trio/trio/pull/2947?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/\_tests/test\_guest\_mode.py](https://app.codecov.io/gh/python-trio/trio/pull/2947?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-trio#diff-c3JjL3RyaW8vX2NvcmUvX3Rlc3RzL3Rlc3RfZ3Vlc3RfbW9kZS5weQ==) | `100.00% <100.00%> (ø)` | | | [src/trio/\_tests/test\_scheduler\_determinism.py](https://app.codecov.io/gh/python-trio/trio/pull/2947?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-trio#diff-c3JjL3RyaW8vX3Rlc3RzL3Rlc3Rfc2NoZWR1bGVyX2RldGVybWluaXNtLnB5) | `100.00% <100.00%> (ø)` | | | [src/trio/\_tests/test\_threads.py](https://app.codecov.io/gh/python-trio/trio/pull/2947?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-trio#diff-c3JjL3RyaW8vX3Rlc3RzL3Rlc3RfdGhyZWFkcy5weQ==) | `100.00% <100.00%> (ø)` | | ... and [9 files with indirect coverage changes](https://app.codecov.io/gh/python-trio/trio/pull/2947/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-trio)
CoolCat467 commented 5 months ago

At the moment the tests will fail until #2946 gets merged, and even then there will be a failure detailed in https://github.com/astral-sh/ruff/issues/9855 that is a bug.

jakkdl commented 5 months ago

Disclaimer: I'm the one that wrote 99% of the code in flake8-trio.

Ruffs implementation of flake8-trio is only a small set (6 out of ~30) of the full rules: https://github.com/Zac-HD/flake8-trio Although a few of them are handled under other rules.

The plugin is intended for users of trio or anyio*, so some rules might not make sense in trio itself, but can ofc then just disable those.

It is also possible to run flake8-trio as a standalone*, so we don't have to bring back flake8. (because we wanted to implement autofix with libcst, which isn't possible when running with flake8). But there might be reasons to run it through flake8, I don't fully remember.

* yes, both parts of the flake8-trio name are incorrect... https://github.com/Zac-HD/flake8-trio/issues/124#issuecomment-1454034177

mikenerone commented 4 months ago

I just want to mention that ruff's TRIO rules are significantly behind flake8-trio, and produce some false positives as a result. See astral-sh/ruff#9934 and astral-sh/ruff#9935.

CoolCat467 commented 4 months ago

Given that ruff's version has a few issues and other points mentioned, I don't think this particular rule is fit for Trio at the moment.