brentyi / tyro

CLI interfaces & config objects, from types
https://brentyi.github.io/tyro
MIT License
514 stars 27 forks source link

TypeError When Parsing Argument #88

Closed dreamilization closed 11 months ago

dreamilization commented 11 months ago

Version: 0.5.14

tyro.cli() fails for the following part of the code - Source

  gradient_checkpointing_kwargs: dict = field(
      default=None,
      metadata={
          "help": "Gradient checkpointing key word arguments such as `use_reentrant`. Will be passed to `torch.utils.checkpoint.checkpoint` through `model.gradient_checkpointing_enable`."
      },
  )

Exception traceback:

SOME_PATH/lib/python3.9/site-packages/tyro/_resolver.py:311: UserWarning: <class 'int'> does not match any type in Union: [<class 'float'>, <class 'NoneType'>]
  warnings.warn(
SOME_PATH/lib/python3.9/site-packages/tyro/_resolver.py:311: UserWarning: <class 'dict'> does not match any type in Union: [<class 'str'>, <class 'NoneType'>]
  warnings.warn(
Traceback (most recent call last):
  File "SOME_SCRIPT.py", line 69, in <module>
    script_args = tyro.cli(ScriptArguments)
  File "SOME_PATH/lib/python3.9/site-packages/tyro/_cli.py", line 187, in cli
    output = _cli_impl(
  File "SOME_PATH/lib/python3.9/site-packages/tyro/_cli.py", line 374, in _cli_impl
    parser_spec = _parsers.ParserSpecification.from_callable_or_type(
  File "SOME_PATH/lib/python3.9/site-packages/tyro/_parsers.py", line 106, in from_callable_or_type
    field_out = handle_field(
  File "SOME_PATH/lib/python3.9/site-packages/tyro/_parsers.py", line 320, in handle_field
    return ParserSpecification.from_callable_or_type(
  File "SOME_PATH/lib/python3.9/site-packages/tyro/_parsers.py", line 106, in from_callable_or_type
    field_out = handle_field(
  File "SOME_PATH/lib/python3.9/site-packages/tyro/_parsers.py", line 312, in handle_field
    if _fields.is_nested_type(field.type_or_callable, field.default):
  File "SOME_PATH/lib/python3.9/site-packages/tyro/_unsafe_cache.py", line 33, in wrapped_f
    out = f(*args, **kwargs)
  File "SOME_PATH/lib/python3.9/site-packages/tyro/_fields.py", line 224, in is_nested_type
    _try_field_list_from_callable(typ, default_instance),
  File "SOME_PATH/lib/python3.9/site-packages/tyro/_fields.py", line 348, in _try_field_list_from_callable
    return _field_list_from_dict(f, default_instance)
  File "SOME_PATH/lib/python3.9/site-packages/tyro/_fields.py", line 734, in _field_list_from_dict
    if default_instance in MISSING_SINGLETONS or len(cast(dict, default_instance)) == 0:
TypeError: object of type 'NoneType' has no len()

I suspect that this might be due to the type hint is not Optional but dict. Since the default does set it as None, maybe tyro should handle this case as well?

brentyi commented 11 months ago

Thanks for flagging this!

On our end I'm happy to either improve the error message or silently convert the type when the default doesn't match. The latter feels weird, so I'd need to think about it.

But ultimately this looks like an error that should be fixed in the transformers code, right? It seems incorrect to create a field that's annotated as dict but defaults to None.

dreamilization commented 11 months ago

I agree! The code snippet from transformers was recently implemented.

I think some error message can be helpful to figure out what went wrong without letting the user to dive into the source code too much as sometimes these can get pretty nested.

Regarding converting the type, maybe adding one additional check to determine whether the default is None or not can be helpful? I.e. treat that variable as Optional[TYPE] type if the default is None. Again, this will be your call.

brentyi commented 11 months ago

Ok! The newest release should expand the type to Optional[T] after raising a warning.