Add support for os.PathLike

[brentyi]

Addressing #36!

Ideally this could be implemented using #30, but that probably won't be ready for a while. (or ever)

[codecov]

Codecov Report

Base: 99.19% // Head: 99.19% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (3e066c0) compared to base (9269b6f).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

@@           Coverage Diff           @@
##             main      #37   +/-   ##
=======================================
  Coverage   99.19%   99.19%           
=======================================
  Files          21       21           
  Lines        1620     1624    +4     
=======================================
+ Hits         1607     1611    +4     
  Misses         13       13           

Flag	Coverage Δ
unittests	99.19% <100.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
tyro/_instantiators.py	99.03% <100.00%> (+0.01%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[JesseFarebro]

It can get a little more complicated than this if you want to create a custom Path. e.g.,

def test_custom_pathlike():
    class CustomPath(pathlib.PurePosixPath):
        def __new__(cls, *args: Union[str, os.PathLike]) -> "CustomPath":
            return super().__new__(cls, *args)

    def main(x: CustomPath) -> CustomPath:
        return x

    assert tyro.cli(main, args=["--x", "/dev/null"]) == CustomPath("/dev/null")

There are a couple of edge cases:

1. inspect.signature will fail to parse the signature from __new__ even though __init__ doesn't exist on pathlib objects. Edit: It seems I was wrong and inspect.signature does parse this correctly.

2. This will still fail to parse os.PathLike in the union.

To address (1) we could implement a custom signature function that is better behaved than inspect.signature but I don't know how to address (2)?

[brentyi]

1. Do you mind explaining the __new__() override instead of __init__()? I'm not familiar with this pattern / subclassing from pathlib.
2. Unions are recursed into, so I think what I have in this PR should work even if the os.PathLike type is inside of a union. You can scroll through some of the implementation details here:

   tyro/tyro/_instantiators.py

   Lines 384 to 457 in 9269b6f

   	def _instantiator_from_union(
   	typ: TypeForm, type_from_typevar: Dict[TypeVar, TypeForm[Any]]
   	) -> Tuple[Instantiator, InstantiatorMetadata]:
   	options = list(get_args(typ))
   	if NoneType in options:
   	# Move `None` types to the beginning.
   	# If we have `Optional[str]`, we want this to be parsed as
   	# `Union[NoneType, str]`.
   	options.remove(NoneType)
   	options.insert(0, NoneType)
   	# General unions, eg Union[int, bool]. We'll try to convert these from left to
   	# right.
   	instantiators = []
   	metas = []
   	nargs: Union[int, Literal["+"]] = 1
   	first = True
   	for t in options:
   	a, b = _instantiator_from_type_inner(
   	t,
   	type_from_typevar,
   	allow_sequences=True,
   	)
   	instantiators.append(a)
   	metas.append(b)
   	if t is not NoneType:
   	# Enforce that `nargs` is the same for all child types, except for
   	# NoneType.
   	if first:
   	nargs = b.nargs
   	first = False
   	elif nargs != b.nargs:
   	# Just be as general as possible if we see inconsistencies.
   	nargs = "+"
   	metavar: str
   	metavar = _join_union_metavars(map(lambda x: cast(str, x.metavar), metas))
   	def union_instantiator(strings: List[str]) -> Any:
   	metadata: InstantiatorMetadata
   	errors = []
   	for i, (instantiator, metadata) in enumerate(zip(instantiators, metas)):
   	# Check choices.
   	if metadata.choices is not None and any(
   	x not in metadata.choices for x in strings
   	):
   	errors.append(
   	f"{options[i]}: {strings} does not match choices {metadata.choices}"
   	)
   	continue
   	# Try passing input into instantiator.
   	if len(strings) == metadata.nargs or (metadata.nargs == "+"):
   	try:
   	return instantiator(strings) # type: ignore
   	except ValueError as e:
   	# Failed, try next instantiator.
   	errors.append(f"{options[i]}: {e.args[0]}")
   	else:
   	errors.append(
   	f"{options[i]}: input length {len(strings)} did not match expected"
   	f" argument count {metadata.nargs}"
   	)
   	raise ValueError(
   	f"no type in {options} could be instantiated from"
   	f" {strings}.\n\nGot errors: \n- " + "\n- ".join(errors)
   	)
   	return union_instantiator, InstantiatorMetadata(
   	nargs=nargs,
   	metavar=metavar,
   	choices=None,
   	)

   
   (the _instantiator_from_type_inner() method may be poorly named)

[JesseFarebro]

1. __new__ creates an instance of the class and __init__ initializes it. How pathlib uses this is that they specialize the implementation in __new__, here's the actual implementation:

class PurePath(object):
    def __new__(cls, *args):
        if cls is PurePath:
            cls = PureWindowsPath if os.name == 'nt' else PurePosixPath
        return cls._from_parts(args)

So depending on the platform it'll actually create a different object. It doesn't utilize __init__ and instead opts to call the class method _from_parts. I was actually mistaken and inspect.signature will properly parse __new__ so ignore my previous comment.

The problem here is actually around creating the instantiator:

tyro/tyro/_instantiators.py

Lines 141 to 170 in 9269b6f

	param_count = 0
	has_var_positional = False
	try:
	signature = inspect.signature(typ)
	except ValueError:
	# No signature, this is often the case with pybind, etc.
	signature = None
	if signature is not None:
	# Some checks we can do if the signature is available!
	for i, param in enumerate(signature.parameters.values()):
	if i == 0 and param.annotation not in (str, inspect.Parameter.empty):
	raise UnsupportedTypeAnnotationError(
	f"Expected {typ} to be an `(arg: str) -> T` type converter, but"
	f" got {signature}."
	)
	if param.kind is inspect.Parameter.VAR_POSITIONAL:
	has_var_positional = True
	elif param.default is inspect.Parameter.empty and param.kind is not (
	inspect.Parameter.VAR_KEYWORD
	):
	param_count += 1
	# Raise an error if parameters look wrong.
	if not (param_count == 1 or (param_count == 0 and has_var_positional)):
	raise UnsupportedTypeAnnotationError(
	f"Expected {typ} to be an `(arg: str) -> T` type converter, but got"
	f" {signature}. You may have a nested type in a container, which is"
	" unsupported."
	)

Tyro needs to take into account not only signatures that are (arg: str) -> T but also (arg: Union[str, Any] -> T (in this case Union[str, os.PathLike]) which is the case with the custom Path class I posted above. The reason this doesn't fail for pathlib is that they don't annotate the arguments to __new__ and in that case, Tyro will just assume it can be called with a string. If one day they annotate pathlib this will also fail.

2. The problem is with the instantiator, sorry for not making this clear.

[JesseFarebro]

It seems this could be solved with something as simple as (logic could be made simpler, just a POC):

_instantiators.py L152

if i == 0 and not (
    (
        get_origin(param.annotation) is Union
        and str in get_args(param.annotation)
    )
    or param.annotation in (str, inspect.Parameter.empty)
):
    raise UnsupportedTypeAnnotationError(
        f"Expected {typ} to be an `(arg: str) -> T` type converter, but"
        f" got {signature}."
    )

[JesseFarebro]

I think you can merge this and I can rebase #38. #38 should solve the other issues I mentioned here.