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

[rbren]

Going to wait for @li-boxuan to take a look at this since he worked through some tricky linting issues w/ flake8 recently

[li-boxuan]

While I appreciate your attempt to reduce the complexity of the pre-commit hooks (which I would love to do too), I am pretty sure flake8 is not covered by Ruff. Autopep8 might be, I need to check.

To prove that flake8 is not covered by Ruff, you could find anything like

f'some string'

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

Now if you remove flake8 (as what you did in this PR), you would see no complaint. Ruff would approve the change. Now we would start the 'single-quote' v.s. 'double-quote' war again because some developers' linters prefer single quotes and others prefer double quotes.

[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

[li-boxuan]

I just cloned your branch, and applied the following patch:

diff --git a/agenthub/monologue_agent/utils/monologue.py b/agenthub/monologue_agent/utils/monologue.py
index 8dfb4ce..88c414b 100644
--- a/agenthub/monologue_agent/utils/monologue.py
+++ b/agenthub/monologue_agent/utils/monologue.py
@@ -75,4 +75,4 @@ class Monologue:
             self.thoughts = prompts.parse_summary_response(summary_resp)
         except Exception as e:
             traceback.print_exc()
-            raise RuntimeError(f'Error condensing thoughts: {e}')
+            raise RuntimeError(f"Error condensing thoughts: {e}")

I was able to commit without issues. On main, however, pre-commit hook will fail the commit to enforce single quotes (which is what we want).

You probably want to incorporate the changes suggested by @dorbanianas.

[Borda]

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

[li-boxuan]

@Borda This is neat, but... 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.

[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?

[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 - astral-sh/ruff#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.

[li-boxuan]

Glad you sorted it out eventually! I am happy to see fewer linters/plugins.

C.C. @rbren

[li-boxuan]

It just came to my mind that you also need to change Makefile, specifically, the following snippet, since your PR decides to lint all files instead of just two folders:

lint:
	@echo "$(YELLOW)Running linters...$(RESET)"
	@poetry run pre-commit run --files opendevin/**/* agenthub/**/* --show-diff-on-failure --config $(PRECOMMIT_CONFIG_PATH)

[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 🐿️