Skip to content

Add level and msg type hints to MultiProcessAdapter.log - #4125

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

Add level and msg type hints to MultiProcessAdapter.log#4125
RudrenduPaul wants to merge 1 commit into
huggingface:mainfrom
RudrenduPaul:add-type-hints-logging

Conversation

@RudrenduPaul

Copy link
Copy Markdown

What this PR does

Adds type hints to the level and msg parameters of
MultiProcessAdapter.log() in src/accelerate/logging.py:

def log(self, level: int, msg: object, *args, **kwargs):

MultiProcessAdapter subclasses logging.LoggerAdapter, and this matches
the standard library's own typeshed signature for
LoggerAdapter.log(self, level: int, msg: object, ...)level is always
an integer logging level (e.g. logging.INFO), and msg is typed as
object rather than str because Python's logging module accepts any
object with a __str__ (including lazy-formatting helper objects), and
that's exactly how this method is exercised elsewhere via
self.isEnabledFor(level) and self.process(msg, kwargs).

This is a self-identified typing-consistency follow-up (no linked issue) —
a small companion to my other open PR (#4123, type hints in
utils/imports.py), which touches a different file in the same repo.

No behaviour change

Type-hint-only change. No logic was modified. The file already has
from __future__ import annotations at the top, so this adds no runtime
overhead.

Testing

  • ruff check src/accelerate/logging.py — 1 pre-existing finding
    (unrelated functools.lru_cache on warning_once), not on the changed
    line, not introduced by this change (verified against upstream main
    before/after via git stash).
  • ruff format --diff src/accelerate/logging.py — no diff, file already
    formatted.

Note: Claude Code was used to assist in drafting this change. All
changes were reviewed by the submitter.

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.

1 participant