Skip to content
Open
Changes from 1 commit
Commits
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
9 changes: 9 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,15 @@ please follow these guidelines:
- **Exception**: Explicit re-export patterns like `from ... import X as X` or marked with "# For export"
- This prevents circular imports and makes dependencies clear

## Python Import Style Guidelines

* **Default to Module-Qualified Imports:** Prefer importing whole modules and using qualified calls (e.g., `import math; math.sqrt(16)` or `import pandas as pd; pd.DataFrame()`) to prevent namespace pollution, avoid name clashes, and provide immediate context for where functions or objects originate.
* **Use Direct Symbol Imports Cautiously:** Restrict direct imports (`from module import symbol`) to specific scenarios where they genuinely improve readability or adhere to standard conventions:
* Importing classes, exceptions, or constants (e.g., `from my_project.models import User`).
* Avoiding severe, repetitive visual clutter in heavy mathematical or algorithmic code.
* Standard library patterns (e.g., `from collections import defaultdict, Counter`).
* **Prohibit Wildcard Imports:** Never use wildcard imports (`from module import *`) under any circumstances.

Comment on lines +123 to +131

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I believe ruff has some rules that enforce this, or at the very least, can be enabled, these sort of things should be deterministic and triggered by pre-commit / CI linting, it's context engineering best practices.

@bmerkle Bernhard Merkle (bmerkle) Aug 26, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fair point, and for context this isn't new ground — I raised almost this exact question in #116 ("consider ruff as alternative to black and isort etc"). Guido van Rossum (@gvanrossum) and I went back and forth on it there (Ruff's isort-compatible config, mixed import/from ordering, etc.), and I closed it once #132 landed a working isort profile that covers the ordering piece we needed at the time: "via #132 we have now a working isort profile in place, so IMO we do not need to consider ruff further, at least for now."

That said, your comment is really about a narrower and separate gap: isort only sorts/groups imports, it doesn't ban wildcard imports or enforce the qualified-vs-direct-import heuristic documented here. Neither of those was in scope of the #116 discussion. The wildcard-ban part is genuinely a one-line, zero-config win with ruff (F403/F405 are in its default rule set) — the qualified-vs-direct heuristic isn't something a standard rule enforces automatically (it needs to distinguish "is this a class/exception/constant", which isn't purely mechanical).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

actually I just remembered that I had the same conversation with guido at some point and he didn't change his mind either.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM with that in mind

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Went ahead and added this: ruff is now a dev dependency with a make ruff target (63c62d8), scoped to F403/F405 (wildcard-import ban) since that's the one piece of this guideline that's genuinely mechanical to enforce. interfaces.py's sanctioned re-export aggregator is carved out via a per-file-ignore, and I tightened the AGENTS.md wildcard-import bullet to explicitly name that exception so the doc and the linter agree.

Not wired into make all/CI yet — following the same staged approach Guido van Rossum (@gvanrossum) suggested back in #116 ("make a small PR that allows us to run make ruff, adding it to CI is a separate step"). Let me know if you'd like CI wiring folded into this PR too, or tracked as the immediate next step.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Kevin Turcios (@KRRT7) I have added first ruff integration, so we can run it locally and proceed incrementally.
Thanks for bringing up the idea again :-)

* Order imports alphabetically after lowercasing; group them as follows
(with a blank line between groups):
1. standard library imports
Expand Down
Loading