[type validation] skip unresolved forward ref

[zucchini-nlp]

As per title, skip if the typing hint is a ForwardRef. In transformers we have cases when the type hints are defined with safe guards, which we can't always resolve. It is safer to keep them as ForwardRef in those cases and skip strict validation

If we want those fields to be checked strictly, we will add a custom validation function with safe imports in transformers. This PR will unblock huggingface/transformers#40793, where some image typing hints require Pillow/torch

cc @gante

[HuggingFaceDocBuilderDev]

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

[gante]

The clause above, isinstance(expected_type, type), is True when expected_type is str. This means or isinstance(expected_type, str) can be removed, correct?

[zucchini-nlp]

not really. The above clause is triggered if exp_type = <string> and the one I added is for cases such as exp_type = "torch.tensor"

In other words, the type is not resolved and remains in string version

[gante]

ah, derp, of course -- expected_type is a string instance here, not the type string 🤦

[gante]

(LGTM, but tagging a proper reviewer: @Wauplin )

[Wauplin]

Seems reasonable! Could you add a unit test in ./tests/test_utils_strict_dataclass.py for it? Would be good to test both with a ForwardRef and with a string ref.

(note: failing CI is unrelated)

[zucchini-nlp]

Failing tests do not look related, so ig this one is ready @Wauplin

[Wauplin]

Instead of modifying existing tests I would prefer that you keep everything as it is + add a new class ConfigWithForwardRef specifically for this PR. Also you are testing on dtype: Union[ForwardRef("torch.dtype"), str] = "float32" which is not really conclusive since even without the forward ref, "float32" is already a valid string value (so str allows it).

So can you create a config like this:

@strict
@dataclass
class ConfigWithForwardRef:
    explicitForwardRef: ForwardRef("torch.dtype")
    implicitForwardRef: "torch.dtype"

and test both cases?

[zucchini-nlp]

@Wauplin oke, made the test which would fail on main branch. We will test only "forwardRef" and "forwardref" with manual validation fn

[Wauplin]

I pushed a last commit. Let's merge it now. Thanks @zucchini-nlp!

[Wauplin]

@zucchini-nlp @gante made a 0.35.1 release for you: https://github.com/huggingface/huggingface_hub/releases/tag/v0.35.1

[zucchini-nlp]

Amazing, thanks a lot!