-
Notifications
You must be signed in to change notification settings - Fork 764
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
chore: remove obsolete flake8 config #2066
Conversation
The only configuration setting in the file was the line length. Do you want to use the same setting with ruff? |
Thanks for the PR! I personally like having a bit longer line length to make certain code a bit more readable (like pandas operations). Having said that, I'm okay either way. Do you have a preference having seen the code in this repo? |
I prefer longer line length as well. 80 or 88 like PEP8 or black recommend/enforce are too narrow for me. I usually prefer 100 or 120. 160 might be too long. What do you think? If you add [tool.ruff]
line-length = 120 the Its your call, but as you asked me, I suggest 120. |
Great, I had a feeling I wasn't the only one who liked a bit longer line length here. Let's do 120 since 160 was a bit of a stretch and 100 is too similar to what we have now. |
Great, LGTM! Should we also re-run ruff to deal with the increased line length or perhaps in a different PR? |
I took the liberty to remove |
Re-ran ruff, lint job is passing. |
@afuetterer I just merged a PR that involves simplifying and improving the zero-shot topic modeling approach. It seems that that merge just created a bunch of conflicts for you, sorry! |
Will fix those soon. |
Rebased, re-ran ruff format on changed files. |
@afuetterer Great, thanks for the work on this! |
What does this PR do?
See #2033 (comment)
Before submitting