Skip to content

chore: enable isort #28

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.

Sign up for GitHub

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

Merged
gabe-l-hart merged 8 commits into from
Mar 20, 2026
Merged

chore: enable isort #28

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ec5f887
chore: fix isort
Mar 12, 2026
16d1c1b
update: added isort back into pre-commit hook
Mar 13, 2026
8f397f7
removed todo
Mar 13, 2026
aa63628
chore: isort fix updates
Mar 13, 2026
71c6570
Update cforge/commands/server/run.py
czhao314 Mar 13, 2026
8c779be
chore: update isort config
Mar 19, 2026
c32c15a
removed vs commented out changes in config
Mar 19, 2026
2ba4cec
fix: Remove PR_ISORT.md
gabe-l-hart Mar 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions .pre-commit-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -421,13 +421,13 @@ repos:
description: The uncompromising Python code formatter.
language_version: python3

# - repo: https://github.com/pycqa/isort
# rev: 6.0.1
# hooks:
# - id: isort
# name: 🐍 isort - Import Sorter
# description: A Python utility / library to sort imports.
# args: [--profile=black]
- repo: https://github.com/pycqa/isort
rev: 6.0.1
hooks:
- id: isort
name: 🐍 isort - Import Sorter
description: A Python utility / library to sort imports.
args: [--profile=black]

# - repo: https://github.com/asottile/pyupgrade
# rev: v3.20.0
Expand Down
1 change: 1 addition & 0 deletions cforge/commands/server/run.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ def run(

# Import top-level translate here to avoid undesirable initialization
# Third Party
# First-Party

@gabe-l-hart gabe-l-hart Mar 13, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
# Third Party
# First-Party
# First-Party

@gabe-l-hart gabe-l-hart Mar 17, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We still need to remove the duplicate # Third Party heading

from mcpgateway.translate import main as translate_main

# Launch the translation wrapper in a subprocess
Expand Down
2 changes: 2 additions & 0 deletions cforge/common/console.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@
configuration without repeated construction.
"""

# Standard
from functools import lru_cache

# Third-Party
from rich.console import Console
import typer

Expand Down
4 changes: 4 additions & 0 deletions cforge/common/errors.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,15 @@
boundary between internal failures and surfaced command errors.
"""

# Standard
from enum import Enum
import json
from typing import Any, Optional, Tuple

# Third-Party
import typer

# First-Party
from cforge.common.console import get_console


Expand Down Expand Up @@ -56,6 +59,7 @@ def split_exception_details(exception: Exception) -> Tuple[str, Any]:

def handle_exception(exception: Exception) -> None:
"""Handle an exception and print a friendly error message."""
# First-Party
from cforge.common.render import print_json

e_str, e_detail = split_exception_details(exception)
Expand Down
3 changes: 3 additions & 0 deletions cforge/common/http.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,14 @@
helpers instead of handling auth headers and base URL resolution themselves.
"""

# Standard
from pathlib import Path
from typing import Any, Dict, Optional

# Third-Party
import requests

# First-Party
from cforge.common.errors import AuthenticationError, CLIError
from cforge.config import get_settings
from cforge.credential_store import load_profile_credentials
Expand Down
3 changes: 3 additions & 0 deletions cforge/common/prompting.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,16 @@
commands that need structured request payloads.
"""

# Standard
import json
from typing import Annotated, Any, Callable, Dict, get_args, get_origin, get_type_hints, List, Optional, Tuple, Union

# Third-Party
from pydantic import BaseModel
from rich.console import Console
import typer

# First-Party
from cforge.common.console import get_console
from cforge.common.errors import CLIError
from cforge.common.schema_validation import validate_instance, validate_instance_against_subschema, validate_schema
Expand Down
3 changes: 3 additions & 0 deletions cforge/common/render.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,19 @@
data retrieval while sharing a consistent terminal presentation.
"""

# Standard
import json
from typing import Any, Dict, List, Optional

# Third-Party
from rich.console import Console, ConsoleOptions, RenderableType, RenderResult
from rich.measure import Measurement
from rich.panel import Panel
from rich.segment import Segment
from rich.syntax import Syntax
from rich.table import Table

# First-Party
from cforge.common.console import get_console
from cforge.config import get_settings

Expand Down
4 changes: 2 additions & 2 deletions cforge/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,16 @@
# Standard
from contextlib import contextmanager
from functools import lru_cache
import os

@gabe-l-hart gabe-l-hart Mar 13, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This looks wrong. isort should keep from ... imports separate from import ... imports.

@czhao314 czhao314 Mar 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

**ignore my previous comment, I see what you mean, import os is in the middle of the from imports; for some reason this does pass the isort --check-only command - will take a look

@gabe-l-hart gabe-l-hart Mar 13, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Typically, I've used imports like the following:

from baz import bat
from foo import bar
# ^ alpha sort for all `from ...` imports
import asdf
import qwer
# ^ alpha sort for all `import ...` imports

(but without the editorial comments in between)

@czhao314 czhao314 Mar 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

gotcha- i found the issue:
there was one style param force_sort_within_sections = true (sort the imports by module, independent of import style) which was overriding from_first= true which does the sorting you mentioned

I got rid of that, then added force_alphabetical_sort_within_sections = true to sort each by alphabetical order

from pathlib import Path
from typing import Generator, Optional, Self
import os

# Third-Party
from pydantic import model_validator

# First-Party
from mcpgateway.config import Settings
from mcpgateway.config import get_settings as cf_get_settings
from mcpgateway.config import Settings

HOME_DIR_NAME = ".contextforge"
DEFAULT_HOME = Path.home() / HOME_DIR_NAME
Expand Down
2 changes: 1 addition & 1 deletion cforge/credential_store.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
# Third-Party
from cryptography.hazmat.backends import default_backend
from cryptography.hazmat.primitives import hashes
from cryptography.hazmat.primitives.ciphers import Cipher, algorithms, modes
from cryptography.hazmat.primitives.ciphers import algorithms, Cipher, modes

@gabe-l-hart gabe-l-hart Mar 17, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It looks like this is doing case-insensitive alphabetical sort. In other projects, I've used case-sensitive so all capitalized come first (ALL_CAPS -> CamelCaps -> lower). It's not a setting I have a strong preference about, but I think that's maybe more standard?

@czhao314 czhao314 Mar 19, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

sounds good, changed it to case sensitive (set case_sensitive = true in config)

from cryptography.hazmat.primitives.kdf.pbkdf2 import PBKDF2HMAC

# First-Party
Expand Down
9 changes: 2 additions & 7 deletions cforge/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
# Third-Party
import typer

# First-Party
from cforge.commands.deploy.deploy import deploy
from cforge.commands.metrics.metrics import metrics_get, metrics_reset
from cforge.commands.resources.a2a import (
Expand All @@ -47,11 +48,7 @@
mcp_servers_toggle,
mcp_servers_update,
)
from cforge.commands.resources.plugins import (
plugins_get,
plugins_list,
plugins_stats,
)
from cforge.commands.resources.plugins import plugins_get, plugins_list, plugins_stats
from cforge.commands.resources.prompts import (
prompts_create,
prompts_delete,
Expand Down Expand Up @@ -101,8 +98,6 @@
from cforge.commands.settings.support_bundle import support_bundle
from cforge.commands.settings.version import version
from cforge.commands.settings.whoami import whoami

# First-Party
from cforge.common.console import get_app

# Get the main app singleton
Expand Down
6 changes: 3 additions & 3 deletions cforge/profile_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@

# Standard
from datetime import datetime
import json
from pathlib import Path
from typing import Dict, List, Optional
import json

# Third-Party
from pydantic import BaseModel, Field, ValidationInfo, field_validator
from pydantic import BaseModel, Field, field_validator, ValidationInfo

# Local
# First-Party

@gabe-l-hart gabe-l-hart Mar 13, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think we want cforge to be Local, not First-Party

@czhao314 czhao314 Mar 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

checked and cforge was set as a known first party in the isort config in the toml, I'll adjust it to known local package!

@gabe-l-hart gabe-l-hart Mar 13, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yeesh, this is what I get for copying all this config from the upstream project rather than starting from scratch!

from cforge.config import get_settings

# Virtual default profile ID for local development
Expand Down
5 changes: 2 additions & 3 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,9 +171,8 @@ target-version = "py311"

[tool.ruff.lint]
# Enable Pyflakes (`F`) and a subset of the pycodestyle (`E`) codes by default.
# Also "D1" for docstring present checks.
# TODO: Enable "I" for import sorting as a separate PR.
select = ["E3", "E4", "E7", "E9", "F", "D1"]
# Also "D1" for docstring present checks and "I" for import sorting.
select = ["E3", "E4", "E7", "E9", "F", "D1", "I"]

@gabe-l-hart gabe-l-hart Mar 13, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm not clear whether it's this change or the one above to enable the pre-commit hook that actually caused all the changes. I'm also concerned that ruff's I might differ from isort w/ --profile=black.

Here's an example of the isort config that I'd like to use for this project: https://github.com/caikit/caikit/blob/main/.isort.cfg. I think there's a way to add this config directly in pyproject.toml without an independent .isort.cfg file which would be the preferred method.

@czhao314 czhao314 Mar 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

it was the change to enable the isort pre-commit hook that caused these changes. i do see ruff's I is causing some sort of issue in the pre-hook.. just tried it again and it fails the black python reformatter and the isort formatting; I'll re-disable it
I think earlier either I might have run the prehook and the "I" didn't save properly so I didn't see any problems, which is completely my bad

ignore = []

# Allow fix for all enabled rules (when `--fix`) is provided.
Expand Down
4 changes: 3 additions & 1 deletion tests/commands/resources/test_a2a.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@

# Standard
import json
import tempfile
from pathlib import Path
import tempfile

@gabe-l-hart gabe-l-hart Mar 13, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Same here, I think the isort settings must be a bit off from what I'm used to

from unittest.mock import patch

# Third-Party
Expand All @@ -28,6 +28,8 @@
a2a_toggle,
a2a_update,
)

# Local
from tests.conftest import patch_functions


Expand Down
4 changes: 3 additions & 1 deletion tests/commands/resources/test_mcp_servers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@

# Standard
import json
import tempfile
from pathlib import Path
import tempfile
from unittest.mock import patch

# Third-Party
Expand All @@ -27,6 +27,8 @@
mcp_servers_toggle,
mcp_servers_update,
)

# Local
from tests.conftest import patch_functions


Expand Down
2 changes: 2 additions & 0 deletions tests/commands/resources/test_plugins.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
from cforge.commands.resources.plugins import _parse_plugin_mode, PluginMode, plugins_get, plugins_list, plugins_stats
from cforge.common.errors import AuthenticationError, CLIError
from cforge.main import app

# Local
from tests.conftest import invoke_typer_command, patch_functions


Expand Down
4 changes: 3 additions & 1 deletion tests/commands/resources/test_prompts.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@

# Standard
import json
import tempfile
from pathlib import Path
import tempfile
from unittest.mock import patch

# Third-Party
Expand All @@ -28,6 +28,8 @@
prompts_toggle,
prompts_update,
)

# Local
from tests.conftest import patch_functions


Expand Down
4 changes: 3 additions & 1 deletion tests/commands/resources/test_resources.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@

# Standard
import json
import tempfile
from pathlib import Path
import tempfile
from unittest.mock import patch

# Third-Party
Expand All @@ -28,6 +28,8 @@
resources_toggle,
resources_update,
)

# Local
from tests.conftest import patch_functions


Expand Down
4 changes: 3 additions & 1 deletion tests/commands/resources/test_tools.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@

# Standard
import json
import tempfile
from pathlib import Path
import tempfile
from unittest.mock import patch

# Third-Party
Expand All @@ -28,6 +28,8 @@
tools_toggle,
tools_update,
)

# Local
from tests.conftest import patch_functions


Expand Down
4 changes: 3 additions & 1 deletion tests/commands/resources/test_virtual_servers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@

# Standard
import json
import tempfile
from pathlib import Path
import tempfile
from unittest.mock import MagicMock, patch

# Third-Party
Expand All @@ -33,6 +33,8 @@
virtual_servers_tools,
virtual_servers_update,
)

# Local
from tests.conftest import mock_mcp_server_sse, patch_functions, register_mcp_server


Expand Down
5 changes: 4 additions & 1 deletion tests/commands/server/test_run.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@

# First-Party
from cforge.commands.server.run import run

# Local
from tests.conftest import invoke_typer_command


Expand Down Expand Up @@ -502,6 +504,7 @@ def test_run_register_without_source_warns(self) -> None:

def test_run_health_check_connection_error_retry(self) -> None:
"""Test that health check retries on connection errors."""
# Third-Party
import requests as real_requests

with (
Expand Down Expand Up @@ -542,8 +545,8 @@ def test_run_health_check_connection_error_retry(self) -> None:

def test_run_health_check_timeout(self) -> None:
"""Test that health check timeout exits with error."""
# Third-Party
import requests as real_requests

import typer

with (
Expand Down
4 changes: 3 additions & 1 deletion tests/commands/server/test_serve.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,17 @@
"""

# Standard
from unittest.mock import patch
import threading
import time
from unittest.mock import patch

# Third-Party
import requests

# First-Party
from cforge.commands.server.serve import serve

# Local
from tests.conftest import get_open_port, invoke_typer_command


Expand Down
2 changes: 1 addition & 1 deletion tests/commands/settings/test_config_schema.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@

# Standard
import json
import tempfile
from pathlib import Path
import tempfile
from unittest.mock import patch

# First-Party
Expand Down
Loading