Skip to content

Add type hints to hotswap utility functions in hotswap.py - #3452

Closed
RudrenduPaul wants to merge 1 commit into
huggingface:mainfrom
RudrenduPaul:add-type-hints-hotswap
Closed

RudrenduPaul wants to merge 1 commit into
huggingface:mainfrom
RudrenduPaul:add-type-hints-hotswap

Conversation

@RudrenduPaul

Copy link
Copy Markdown
Contributor

What does this PR do?

Adds missing type annotations to utility functions in src/peft/utils/hotswap.py, following the same pattern as the already-merged type-hint cleanup in #3144.

The following functions were missing type hints:

hotswap.py

  • _update_scaling: added LoraLayer (param lora_module), str (param adapter_name), Optional[float] = None (param scaling), and None return type
  • hotswap_adapter_from_state_dict: added None return type (params were already annotated)
  • hotswap_adapter: added torch.nn.Module (param model), str (param model_name_or_path), str (param adapter_name), Optional[str] = None (param torch_device), and None return type

prepare_model_for_compiled_hotswap was also checked per the task scope, but it already had complete type hints on every parameter and the return type, so it was left unchanged — no genuinely missing hints there.

Type choices were verified against real call sites, not just docstrings:

  • _update_scaling's lora_module param is typed as LoraLayer (not the more generic torch.nn.Module used elsewhere in this file) because the function body accesses lora_module.scaling, a dict attribute defined on LoraLayer, not on torch.nn.Module. Typing it as torch.nn.Module produced 14 new pyright errors inside the function body (subscript/attribute access on an untyped Module.__getattr__ fallback); typing it as LoraLayer eliminates all of them while accurately describing the real runtime objects passed in (Linear/Conv2d LoRA layers, both of which subclass LoraLayer).
  • hotswap_adapter's model_name_or_path param is typed as str (matching the declared signatures of PeftConfig.from_pretrained and load_peft_weights, both of which it's passed into) rather than str | os.PathLike, even though some tests pass pathlib.Path objects — widening the annotation to include os.PathLike caused a new pyright error at the load_peft_weights(model_name_or_path, ...) call site, since that function's own parameter is declared as str only.

Tests

Type-hint only change — no behavioral modification.

  • python -m py_compile src/peft/utils/hotswap.py — passes
  • ruff check / ruff format --check (pinned version 0.15.12) — both pass with no findings
  • npx pyright src/peft/utils/hotswap.py — error count and messages are byte-for-byte identical before and after this change (56 errors both times, all pre-existing, in code untouched by this PR, or due to import resolution in a bare env without torch/accelerate fully installed — none introduced by the newly annotated functions)

Before submitting

AI assistance disclosure

This PR was developed with the assistance of Claude Code (AI). All changes have been read, understood, and verified by the human contributor (Rudrendu Paul), including cross-referencing every added type against the function's real call sites and confirming via pyright that no new type errors were introduced.

Adds missing type annotations to _update_scaling, hotswap_adapter_from_state_dict,
and hotswap_adapter in src/peft/utils/hotswap.py, following the same pattern as the
already-merged type-hint cleanup in huggingface#3144. prepare_model_for_compiled_hotswap was
checked and already had complete type hints, so it was left unchanged.
@BenjaminBossan

Copy link
Copy Markdown
Member

If you're working on multiple PRs to add type hints, I would strongly prefer you could pool them into a single PR, even if multiple files are being touched.

@RudrenduPaul

Copy link
Copy Markdown
Contributor Author

Thanks for the feedback @BenjaminBossan — pooled this change together with #3448 into a single consolidated PR: #3529. Closing this one in favor of that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants