Replace flake8 and pylint with ruff #416
Conversation
|
Coverage after merging jb/use-ruff into master will be
Coverage Report for Changed Files
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
jongbinjung
force-pushed
the
jb/use-ruff
branch
from
37f1ea2
to
c14d1a6
Compare
|
Coverage after merging jb/use-ruff into master will be
Coverage Report for Changed Files
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Coverage after merging jb/use-ruff into master will be
Coverage Report for Changed Files
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Coverage after merging jb/use-ruff into master will be
Coverage Report for Changed Files
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Coverage after merging jb/use-ruff into master will be
Coverage Report for Changed Files
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Coverage after merging jb/use-ruff into master will be
Coverage Report for Changed Files
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "def plot_shape_and_cells(shape, res=9):\n", | ||
| " fig, axs = plt.subplots(1,2, figsize=(10,5), sharex=True, sharey=True)\n", |
| " (37.778, -122.507),\n", | ||
| " (37.733, -122.501)\n", | ||
| "]\n", | ||
| "outer = [(37.804, -122.412), (37.778, -122.507), (37.733, -122.501)]\n", |
There was a problem hiding this comment.
The way a lot of these auto formatters work is that you can add a trailing comma to the last element to force it to have one element per line.
| @@ -429,7 +433,13 @@ | |||
| "poly1 = h3.LatLngPoly([(37.804, -122.412), (37.778, -122.507), (37.733, -122.501)])\n", | |||
| "poly2 = h3.LatLngPoly(\n", | |||
| " [(37.803, -122.408), (37.736, -122.491), (37.738, -122.380), (37.787, -122.39)],\n", | |||
| " [(37.760, -122.441), (37.772, -122.427), (37.773, -122.404), (37.758, -122.401), (37.745, -122.428)]\n", | |||
There was a problem hiding this comment.
I was about to complain about this, but its inconsistent with what I just said above. 😬
| _out_collection, | ||
| _out_scalar, | ||
| ) |
| '892830828c7ffff', | ||
| '892830828d7ffff', | ||
| '8928308289bffff', | ||
| '89283082813ffff', | ||
| '8928308288fffff', | ||
| '89283082883ffff', |
| @@ -400,7 +398,7 @@ def test_multipoly_checks(): | |||
| def test_3d_geo(): | |||
| loop = lnglat_open() | |||
|
|
|||
| loop2d = [(lng, lat) for lng, lat in loop] | |||
| loop2d = [(lng, lat) for lng, lat in loop] | |||
There was a problem hiding this comment.
Why? Is there any benefit of having the for in the two loops aligned?
| [31.57, -94.26], | ||
| [42.94, -89.38], | ||
| [42.68, -110.61], |
There was a problem hiding this comment.
Worse. But i do like that it added the trailing comma :)
| [42.32, -91.80], | ||
| [41.37, -98.61] | ||
| ] | ||
| hole2 = [[41.37, -98.61], [40.04, -91.80], [42.32, -91.80], [41.37, -98.61]] |
There was a problem hiding this comment.
Yes. Adding the trailing comma will auto fmt this to be element-per-line.
| @@ -68,7 +61,7 @@ def test_h3shape_to_cells(): | |||
| '832b1afffffffff', | |||
| '832b1efffffffff', | |||
| '832ba9fffffffff', | |||
| '832badfffffffff' | |||
| '832badfffffffff', | |||
|
I like the idea of Maybe just start with the most important rules?
:P |
There was a problem hiding this comment.
The formatter (cf. linter) which does the automatic formatting isn't really configurable. The linter is, but if we're just going to use the linter and not autofmt, might as well stick with flake8.
But, I think the issues you pointed out are:
- Per-element line breaks for collections that are within line length: This can be forced with trailing commas
- Aligning on the decimal
- Whitespace preference
The latter two—while not really supported—seem to only come up in tests, where there's a lot of personal "touches" to describe/build test data. So, we could just have the formatter skip unit tests?
| " (37.778, -122.507),\n", | ||
| " (37.733, -122.501)\n", | ||
| "]\n", | ||
| "outer = [(37.804, -122.412), (37.778, -122.507), (37.733, -122.501)]\n", |
There was a problem hiding this comment.
The way a lot of these auto formatters work is that you can add a trailing comma to the last element to force it to have one element per line.
| @@ -400,7 +398,7 @@ def test_multipoly_checks(): | |||
| def test_3d_geo(): | |||
| loop = lnglat_open() | |||
|
|
|||
| loop2d = [(lng, lat) for lng, lat in loop] | |||
| loop2d = [(lng, lat) for lng, lat in loop] | |||
There was a problem hiding this comment.
Why? Is there any benefit of having the for in the two loops aligned?
| [42.32, -91.80], | ||
| [41.37, -98.61] | ||
| ] | ||
| hole2 = [[41.37, -98.61], [40.04, -91.80], [42.32, -91.80], [41.37, -98.61]] |
There was a problem hiding this comment.
Yes. Adding the trailing comma will auto fmt this to be element-per-line.
|
OK, so it sounds like what I'm looking for is just using |
* Build 3.13 wheels * bump to pypa/[email protected] * a few more 3.13s * uses: actions/[email protected] * actions/[email protected] * docker/[email protected] * actions/[email protected] * actions/[email protected] * pypa/[email protected] * Steal from #416 * note PR
|
Ruff has started to bring me some joy with #428 |
|
Thanks for this! I stole some ideas and used them in #428 |
In case we're still interested in #337
Tried my best to accommodate existing code style.
Also added review dog to github actions, so that it's easier to see broken things in the PR directly