-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
STYLE: Fix linting of benchmarks #22884
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Using That said, it looks like most are just import errors. |
@TomAugspurger Is it the case with : |
@MatanCohe that's right, and also all the rest in the list that has the error But still, it'd be nice to have the right exception if it's easy, we may unintentionally silent bugs with |
@datapythonista sorry about not being clear about what I'm asking. |
I'm not quite sure what I think the best is to replace the |
lint-adjacent for the benchmarks: I think some of the directives being used are deprecated in asv. e.g. in So the pertinent suggestion: linting for valid asv directives |
That's good to know @jbrockmendel. I think there are some benchmarks that are failing too. I'll see if I can create an issue for them later. Do you want to create an issue for the linting of directives? It'd be nice to get #22863 merged first, as it refactors the whole linting, but I'll fix the conflicts if someone is so fast to implement the linting before. :) |
We are currently not linting (validating PEP-8) of the benchmarks in
asv_bench/benchmarks
. But it'd be good to start doing it as with the rest of the code.Before starting the automatic validation, we need to fix all the style problems. Those can be obtained by executing
flake8 --config=none asv_bench/benchmarks --ignore=F811,E731
This is currently returning:
All them should be fixed. Most of them should be immediate to fix, but for
E722 do not use bare except
make sure the specific exception is captured, and don't simply useexcept Exception:
.The text was updated successfully, but these errors were encountered: