fix(tracker): guard init_trackers and log against None kwargs#4026
Open
xodn348 wants to merge 1 commit into
Open
fix(tracker): guard init_trackers and log against None kwargs#4026xodn348 wants to merge 1 commit into
xodn348 wants to merge 1 commit into
Conversation
Both Accelerator.init_trackers and Accelerator.log accept
`dict | None` for their kwargs arguments but called `.get()` on
the value directly; passing None raised AttributeError. Change
the default from `{}` to None and add an explicit None-guard in
each function body so the type annotation matches the runtime
behaviour.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Accelerator.init_trackersandAccelerator.logboth declare their optional dict arguments asdict | None, but the function bodies call.get()on the value directly without aNoneguard. Passinginit_kwargs=Noneorlog_kwargs=Nonetherefore raisesAttributeError: 'NoneType' object has no attribute 'get'at runtime, even though the type annotation marksNoneas valid. The root cause is that both signatures used= {}as the default (a mutable-default antipattern in Python) while acceptingNonein the union type; the fix changes the default toNoneand adds an explicitif … is None: … = {}guard inside each function body.Issue
No prior issue — found via code review while auditing tracker-related code paths.
Local verification
Output:
=== LOCAL_TEST_PASSED ===
Risk
The change is backward-compatible: callers that already pass a real dict (the common case) see no difference. The only behavior change is that
Noneis now tolerated instead of crashing, which matches what the type annotation already promised. No distributed or multi-GPU code paths are touched.