tskit-dev / tskit

Population-scale genomics
MIT License
147 stars 69 forks source link

Switch to ruff linting #2952

Open benjeffery opened 1 month ago

benjeffery commented 1 month ago

For numpy2 checks etc.

codecov[bot] commented 1 month ago

Codecov Report

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

Project coverage is 86.65%. Comparing base (998d710) to head (54dabad).

:exclamation: Current head 54dabad differs from pull request most recent head 872880c

Please upload reports for the commit 872880c to get more accurate results.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #2952 +/- ## ========================================== - Coverage 89.63% 86.65% -2.99% ========================================== Files 29 11 -18 Lines 30189 22932 -7257 Branches 5877 4254 -1623 ========================================== - Hits 27061 19872 -7189 + Misses 1789 1754 -35 + Partials 1339 1306 -33 ``` | [Flag](https://app.codecov.io/gh/tskit-dev/tskit/pull/2952/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tskit-dev) | Coverage Δ | | |---|---|---| | [c-tests](https://app.codecov.io/gh/tskit-dev/tskit/pull/2952/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tskit-dev) | `86.20% <ø> (ø)` | | | [lwt-tests](https://app.codecov.io/gh/tskit-dev/tskit/pull/2952/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tskit-dev) | `80.78% <ø> (ø)` | | | [python-c-tests](https://app.codecov.io/gh/tskit-dev/tskit/pull/2952/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tskit-dev) | `88.72% <ø> (ø)` | | | [python-tests](https://app.codecov.io/gh/tskit-dev/tskit/pull/2952/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tskit-dev) | `?` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tskit-dev#carryforward-flags-in-the-pull-request-comment) to find out more. [see 18 files with indirect coverage changes](https://app.codecov.io/gh/tskit-dev/tskit/pull/2952/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tskit-dev)
codecov-commenter commented 1 month ago

Codecov Report

Attention: Patch coverage is 98.95833% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 89.63%. Comparing base (998d710) to head (d9256d9).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #2952 +/- ## ========================================== - Coverage 89.63% 89.63% -0.01% ========================================== Files 29 29 Lines 30189 30173 -16 Branches 5877 5877 ========================================== - Hits 27061 27045 -16 Misses 1789 1789 Partials 1339 1339 ``` | [Flag](https://app.codecov.io/gh/tskit-dev/tskit/pull/2952/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tskit-dev) | Coverage Δ | | |---|---|---| | [c-tests](https://app.codecov.io/gh/tskit-dev/tskit/pull/2952/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tskit-dev) | `86.20% <ø> (ø)` | | | [lwt-tests](https://app.codecov.io/gh/tskit-dev/tskit/pull/2952/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tskit-dev) | `80.78% <ø> (ø)` | | | [python-c-tests](https://app.codecov.io/gh/tskit-dev/tskit/pull/2952/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tskit-dev) | `88.72% <ø> (ø)` | | | [python-tests](https://app.codecov.io/gh/tskit-dev/tskit/pull/2952/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tskit-dev) | `99.03% <98.95%> (-0.01%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tskit-dev#carryforward-flags-in-the-pull-request-comment) to find out more. | [Files](https://app.codecov.io/gh/tskit-dev/tskit/pull/2952?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tskit-dev) | Coverage Δ | | |---|---|---| | [python/tskit/cli.py](https://app.codecov.io/gh/tskit-dev/tskit/pull/2952?src=pr&el=tree&filepath=python%2Ftskit%2Fcli.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tskit-dev#diff-cHl0aG9uL3Rza2l0L2NsaS5weQ==) | `96.96% <100.00%> (ø)` | | | [python/tskit/combinatorics.py](https://app.codecov.io/gh/tskit-dev/tskit/pull/2952?src=pr&el=tree&filepath=python%2Ftskit%2Fcombinatorics.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tskit-dev#diff-cHl0aG9uL3Rza2l0L2NvbWJpbmF0b3JpY3MucHk=) | `99.36% <100.00%> (ø)` | | | [python/tskit/drawing.py](https://app.codecov.io/gh/tskit-dev/tskit/pull/2952?src=pr&el=tree&filepath=python%2Ftskit%2Fdrawing.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tskit-dev#diff-cHl0aG9uL3Rza2l0L2RyYXdpbmcucHk=) | `99.23% <100.00%> (-0.01%)` | :arrow_down: | | [python/tskit/exceptions.py](https://app.codecov.io/gh/tskit-dev/tskit/pull/2952?src=pr&el=tree&filepath=python%2Ftskit%2Fexceptions.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tskit-dev#diff-cHl0aG9uL3Rza2l0L2V4Y2VwdGlvbnMucHk=) | `100.00% <100.00%> (ø)` | | | [python/tskit/formats.py](https://app.codecov.io/gh/tskit-dev/tskit/pull/2952?src=pr&el=tree&filepath=python%2Ftskit%2Fformats.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tskit-dev#diff-cHl0aG9uL3Rza2l0L2Zvcm1hdHMucHk=) | `99.46% <100.00%> (ø)` | | | [python/tskit/genotypes.py](https://app.codecov.io/gh/tskit-dev/tskit/pull/2952?src=pr&el=tree&filepath=python%2Ftskit%2Fgenotypes.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tskit-dev#diff-cHl0aG9uL3Rza2l0L2dlbm90eXBlcy5weQ==) | `99.07% <100.00%> (ø)` | | | [python/tskit/intervals.py](https://app.codecov.io/gh/tskit-dev/tskit/pull/2952?src=pr&el=tree&filepath=python%2Ftskit%2Fintervals.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tskit-dev#diff-cHl0aG9uL3Rza2l0L2ludGVydmFscy5weQ==) | `100.00% <100.00%> (ø)` | | | [python/tskit/metadata.py](https://app.codecov.io/gh/tskit-dev/tskit/pull/2952?src=pr&el=tree&filepath=python%2Ftskit%2Fmetadata.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tskit-dev#diff-cHl0aG9uL3Rza2l0L21ldGFkYXRhLnB5) | `99.04% <100.00%> (-0.01%)` | :arrow_down: | | [python/tskit/provenance.py](https://app.codecov.io/gh/tskit-dev/tskit/pull/2952?src=pr&el=tree&filepath=python%2Ftskit%2Fprovenance.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tskit-dev#diff-cHl0aG9uL3Rza2l0L3Byb3ZlbmFuY2UucHk=) | `100.00% <ø> (ø)` | | | [python/tskit/stats.py](https://app.codecov.io/gh/tskit-dev/tskit/pull/2952?src=pr&el=tree&filepath=python%2Ftskit%2Fstats.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tskit-dev#diff-cHl0aG9uL3Rza2l0L3N0YXRzLnB5) | `100.00% <100.00%> (ø)` | | | ... and [5 more](https://app.codecov.io/gh/tskit-dev/tskit/pull/2952?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tskit-dev) | |
benjeffery commented 1 month ago

Well this was a lot noisier than I was expecting, although it did show up a couple of actual errors.

jeromekelleher commented 1 month ago

Ruff is quite opinionated about getting people to use f strings etc isn't it?

What were the errors?

LGTM though, happy to switch.

Given the noisiness we may want to try and flush a few open PRs through first.

benjeffery commented 1 month ago

If we don't want to enforce f-strings we can switch that off - they do generally read better though.

The errors were: A test where a subsequent pytest.raises was nested inside the first, such that the second didn't run. An ibd test that was missing an assert so the value was evaluated, but not checked.

petrelharp commented 1 month ago

I'm happy with whatevery you all think, but I did find two places I wished it was less opinionated (see comments).

hyanwong commented 58 minutes ago

I've been using ruff and it's much quicker than our previous linter, which makes development a little slicker and more palatable, so I would be happy with switching over.