Skip to content

Commit 540ff6d

Browse files
[adapter,docs] refactor: hoist HF imports to module top; codify rule
Move the two `huggingface_hub` imports added in the previous commit out of function bodies and up to the module's import block: - `from huggingface_hub import snapshot_download` in `src/flow_factory/utils/checkpoint.py` - `from huggingface_hub.errors import RepositoryNotFoundError, HfHubHTTPError` in `src/flow_factory/models/abc.py` `huggingface_hub` is already a hard dependency (`pyproject.toml:39`), so there is no import-cost concern; lazy-loading only hid the dependency surface from readers and `isort`. Codify the rule so this pattern is caught in review going forward: - Extend constraint X-GenGroup#22 ("Import Style") in `.agents/knowledge/constraints.md` with an explicit "Top-level imports only" bullet, listing the three sanctioned exceptions (optional deps via `try/except ImportError`, backend-gated imports under runtime feature checks like DeepSpeed/FSDP, and unresolvable circular imports). - Add a matching checklist item to the Code Style section of `.agents/skills/ff-review/SKILL.md`. Pre-existing inline FSDP/DeepSpeed imports in `models/abc.py` (lines ~997-1152) are grandfathered under the new rule's exception (b). Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 2207c92 commit 540ff6d

4 files changed

Lines changed: 4 additions & 4 deletions

File tree

.agents/knowledge/constraints.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,7 @@ The adapter sets inference dtype for frozen components and training dtype for tr
131131
- Use relative imports within `flow_factory` package (e.g., `from ..hparams import *`)
132132
- Use absolute imports for external packages
133133
- Follow existing wildcard import patterns for `hparams`
134+
- **Top-level imports only**: All `import` / `from ... import ...` statements MUST live at the top of the module, never inside function bodies, methods, `__init__`, or conditional branches. Sanctioned exceptions: (a) optional dependencies wrapped in `try/except ImportError` (e.g., `deepspeed`, `xformers`); (b) backend-gated imports where the target symbol is only resolvable under a specific runtime backend already selected by a preceding feature check (e.g., DeepSpeed/FSDP submodules guarded by `is_deepspeed()` / `is_fsdp2()` in `models/abc.py`); (c) genuine unresolvable circular imports documented inline. Lazy imports added merely for "import speed" or "to keep the module light" are NOT acceptable — every hard dependency already runs through Python's import machinery on a typical import path. Inline imports hide the dependency surface from readers, `isort`, and static-analysis tools, and re-execute on every call in hot loops.
134135

135136
### 23. Type Annotations
136137
All public methods must have type annotations. Use `typing` module types (`List`, `Dict`, `Optional`, `Tuple`, `Union`) for Python 3.10 compatibility.

.agents/skills/ff-review/SKILL.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ git status # Modified files
6060
- [ ] English comments and docstrings
6161
- [ ] Apache 2.0 license header on new files
6262
- [ ] No unnecessary wildcard imports (except `hparams`)
63+
- [ ] **Top-level imports only** — no `import` / `from ... import ...` inside function or method bodies (constraint #22). Exceptions: documented optional deps via `try/except ImportError`, or genuine unresolvable circular imports.
6364

6465
### Documentation
6566
- [ ] `guidance/` docs updated if behavior changed

src/flow_factory/models/abc.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
from peft import get_peft_model, LoraConfig, PeftModel
3939

4040
from huggingface_hub import split_torch_state_dict_into_shards
41+
from huggingface_hub.errors import RepositoryNotFoundError, HfHubHTTPError
4142
from accelerate import Accelerator, DistributedType
4243
from accelerate.state import PartialState
4344
from accelerate.utils.modeling import (
@@ -1484,8 +1485,6 @@ def _resolve_checkpoint_path(self, path: str) -> str:
14841485
Raises:
14851486
FileNotFoundError: When the spec is neither a local path nor a reachable HF repo.
14861487
"""
1487-
from huggingface_hub.errors import RepositoryNotFoundError, HfHubHTTPError
1488-
14891488
force_hf = path.startswith(HF_PATH_PREFIX)
14901489
spec = path[len(HF_PATH_PREFIX):] if force_hf else path
14911490

src/flow_factory/utils/checkpoint.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
from typing import Dict, Optional, List, Tuple, Literal
2525

2626
from safetensors.torch import save_file, load_file
27+
from huggingface_hub import snapshot_download
2728

2829
def mapping_lora_state_dict(
2930
state_dict: Dict[str, torch.Tensor],
@@ -223,8 +224,6 @@ def download_hf_checkpoint(
223224
f"expected 'owner/repo' for repo_id, got {repo_id!r}"
224225
)
225226

226-
from huggingface_hub import snapshot_download
227-
228227
allow_patterns: Optional[List[str]] = None
229228
if subfolder:
230229
# Match the subfolder itself plus everything beneath it.

0 commit comments

Comments
 (0)