Skip to content
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

Fix PEP-8 violations reported by flake8 on asv/step_detect.py #1006

Merged

Conversation

LucyJimenez
Copy link
Contributor

@LucyJimenez LucyJimenez commented Feb 2, 2022

closes https://github.com/pandas-benchmarks-grant/board-issues/issues/6
Fix PEP-8 violations reported by flake8 on asv/step_detect.py

@datapythonista
Copy link
Member

For all those l variable errors, what I'd do instead of renaming to l_variable or the noqa, which aren't ideal, you can simply add to the ignore list its error code, and open an issue to take care of them later. This way we can merge all the non-controversial fixes here, and we can have a dedicated issue and PR for that error, where we can focus on that specific problem.

@LucyJimenez
Copy link
Contributor Author

For all those l variable errors, what I'd do instead of renaming to l_variable or the noqa, which aren't ideal, you can simply add to the ignore list its error code, and open an issue to take care of them later. This way we can merge all the non-controversial fixes here, and we can have a dedicated issue and PR for that error, where we can focus on that specific problem.

Hi @datapythonista, thank you for your insight. I'm going to back all the l_variable variables to l and include the error E741 on setup.cfg

@LucyJimenez LucyJimenez marked this pull request as draft February 2, 2022 12:27
@LucyJimenez LucyJimenez marked this pull request as ready for review February 2, 2022 12:55
@LucyJimenez
Copy link
Contributor Author

@datapythonista the PR is ready to review. I also created the issue #1008
All comments are welcome :)

Copy link
Member

@datapythonista datapythonista left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @LucyJimenez looks great.

setup.cfg Outdated
ignore =
W504, # W504: Line break occurred after a binary operator
E741 # E741: Do not use variables named 'I', 'O', or 'l
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a comment with a TODO pointing to the issue you created, so there is more context of why we ignore this error code?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ye, I'll do it!

@datapythonista
Copy link
Member

Can you rebase please?

ignore =
W504, # W504: Line break occurred after a binary operator
E741 # E741: Do not use variables named 'I', 'O', or 'l'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you create an issue to fix the E741 errors on github, and in a follow up PR add a comment above this line with the issue number please.

@datapythonista datapythonista merged commit 357b6e3 into airspeed-velocity:master Feb 7, 2022
@datapythonista
Copy link
Member

Thanks @LucyJimenez for the clean up

@LucyJimenez LucyJimenez deleted the check_flake8_01 branch March 15, 2022 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants