lint: simplify hooks already covered by Ruff

[Borda]

flake8 and autopep8 are already covered by Ruff with --fix argument

[dorbanianas]

Agree with you, I guess it was introduced here #1112 to prevent the single-quote issue related to different python versions, but I guess we can abandon flake8 and autopep8, also can you try to remove the deps from pyproject.toml file

[Borda]

also can you try to remove the deps from pyproject.toml file

done :)

[rbren]

CC @li-boxuan

[li-boxuan]

ChatGPT tells me AutoPep8 is not covered by Ruff: https://chat.openai.com/share/7214f2e7-853c-46dd-844a-5de6af6934da

[dorbanianas]

This for flake8 for the autopep8 I agree that ruff doesn't replace it

[dorbanianas]

@Borda you can check this for to enable single quote by modifying the ruff config file https://docs.astral.sh/ruff/formatter/#__tabbed_1_2

[Borda]

in the codebase, change it to double quotes, and run make lint. You will see flake8 would complain it shall be single quotes.

then it shall be reposted as bug :)

[Borda]

just tested, and by enabling the basic rules like E, W, and F to have parity with flake8, the issues is recognized correctly as F541 [*] f-string without any placeholders

[Borda]

@li-boxuan addressed your comments, mind rechecking it? :flamingo:

[li-boxuan]

@Borda This is neat, but... I tried https://github.com/OpenDevin/OpenDevin/pull/1204#pullrequestreview-2010472657 again on your branch and still the same problem. Looks like ruff isn't able to enforce single quote behavior, or you did something wrong.

[Borda]

I tried #1204 (review) again on your branch and still the same problem. Looks like ruff isn't able to enforce single quote behavior, or you did something wrong.

Hmmm, let me double-check it and eventually report it as an issue...

UPDATE: trying to reproduce, found the line in the codebase including f'Error condensing thoughts: {e}' and ran the line from CI workflow

pre-commit run --files opendevin/**/* agenthub/**/* --show-diff-on-failure --config ./dev_config/python/.pre-commit-config.yaml

And seeing all checks pass without any change means that ruf-format is okay with it...

@li-boxuan, could you pls share the steps to reproduce your case?

[rbren]

Looks like I'm blocking merge on this one, but I'll wait for @li-boxuan to approve. Please @ me once that's done!

[li-boxuan]

And seeing all checks pass without any change means that ruf-format is okay with it.

@Borda You need to change single quotes from double quotes on that line. You would see ruff-format is okay with that change. That's not what we want. We want it to reject the change because they should be single quotes.

In other words, if you push a commit where you change that line from using single quotes to double quotes, we should see CI failure. With your change, CI won't fail. That means people are free to change it to double quotes, and they will (because of linter setting on their side). Then other people would change double quotes back to single quotes for the same reason. We don't want to see this happen.

[Borda]

@li-boxuan addressing it with another pre-commit hook - https://github.com/pre-commit/pre-commit-hooks/blob/main/README.md#double-quote-string-fixer

UPDATED: seems to be fixed with Ruff linter and enabling "Q" rule - https://github.com/astral-sh/ruff/issues/7834

[li-boxuan]

addressing it with another pre-commit hook - https://github.com/pre-commit/pre-commit-hooks/blob/main/README.md#double-quote-string-fixer

This is what we removed earlier, because it doesn't reliably convert double quotes to single quotes when python 3.12 is used.

Could you please test locally with python 3.12 installed?

[Borda]

@li-boxuan sure, that is why I used the Q rule which found several more issues

[li-boxuan]

I generated a patch for testing:

diff --git a/agenthub/codeact_agent/codeact_agent.py b/agenthub/codeact_agent/codeact_agent.py
index 9ff503f..74bdfe2 100644
--- a/agenthub/codeact_agent/codeact_agent.py
+++ b/agenthub/codeact_agent/codeact_agent.py
@@ -108,7 +108,7 @@ class CodeActAgent(Agent):
                         {'role': 'user', 'content': obs.content})
                 elif isinstance(obs, CmdOutputObservation):
                     content = 'OBSERVATION:\n' + obs.content
-                    content += f'\n[Command {obs.command_id} finished with exit code {obs.exit_code}]]'
+                    content += f"\n[Command {obs.command_id} finished with exit code {obs.exit_code}]]"
                     self.messages.append({'role': 'user', 'content': content})
                 else:
                     raise NotImplementedError(

Using python 3.12, I can confirm fix double quoted strings doesn't capture the issue, but ruff does. I am happy now.

[Borda]

@li-boxuan updated in 068d7cd

[Borda]

@rbren, what is the best way to resolve the Poetry lock issue / update it with minimal changes...

[Borda]

@rbren mind approve the CI ru, pls :chipmunk: