Skip to content

hf_argparser: fix TypeError when resolving Optional of generic type#46009

Closed
Dev-X25874 wants to merge 1 commit into
huggingface:mainfrom
Dev-X25874:fix/hf-argparser-optional-generic-isinstance
Closed

hf_argparser: fix TypeError when resolving Optional of generic type#46009
Dev-X25874 wants to merge 1 commit into
huggingface:mainfrom
Dev-X25874:fix/hf-argparser-optional-generic-isinstance

Conversation

@Dev-X25874

Copy link
Copy Markdown

What does this PR do?

Fixes a TypeError in HfArgumentParser._parse_dataclass_field when a dataclass field is typed as Optional[X] where X is a subscripted generic (e.g. Optional[List[str]], Optional[list[str]], Optional[Dict[str, int]]).

The root cause is on line 185, where isinstance(None, field.type.__args__[1]) is used to detect whether the second type argument is NoneType. This works for plain classes like int or str, but raises TypeError: Subscripted generics cannot be used with class and instance checks when args[1] is a generic alias — which happens when None appears first in the union, e.g. Union[None, List[str]].

The fix replaces the isinstance call with an identity check: field.type.__args__[1] is type(None). This is the correct, idiomatic way to test for NoneType and mirrors the pattern already used two lines above (type(None) not in field.type.__args__). It is safe for all types including generics, and produces identical results for plain classes where isinstance happened to work.

Fixes # (issue)

Code Agent Policy

  • I confirm that this is not a pure code agent PR.

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you read the contributor guideline, Pull Request section?
  • Was this discussed/approved via a Github issue or the forum? Please add a link to it if that's the case.
  • Did you make sure to update the documentation with your changes? Here are the documentation guidelines, and here are tips on formatting docstrings.
  • Did you write any new necessary tests?

Who can review?

@ArthurZucker

@github-actions

Copy link
Copy Markdown
Contributor

This PR was flagged by our automated quality checks. If you're a genuine
contributor, please reply here and a maintainer will review your PR.

Common reasons for flagging:

  • New GitHub account
  • Unusually high number of repository forks in a 24-hour window

We appreciate your contribution and apologize if this is a false positive!

@Dev-X25874

Copy link
Copy Markdown
Author

Hi, I understand the automated flag and appreciate the bot catching low-quality submissions — the repo clearly needs it.

To clarify: I found this bug manually by reading through hf_argparser.py while exploring the codebase as part of my open-source learning. The specific issue caught my eye because isinstance(None, X) is a known Python footgun with generics. I verified it by running a local Python snippet before writing a single line of the fix.

I'm happy to answer any questions about the bug or the reasoning behind the fix to demonstrate this. The change is a single expression on line 185 — genuinely as small as it looks.

@Dev-X25874 Dev-X25874 deleted the fix/hf-argparser-optional-generic-isinstance branch May 18, 2026 12:12
@Dev-X25874 Dev-X25874 restored the fix/hf-argparser-optional-generic-isinstance branch May 18, 2026 12:28
@Dev-X25874 Dev-X25874 deleted the fix/hf-argparser-optional-generic-isinstance branch May 28, 2026 05:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants