-
Notifications
You must be signed in to change notification settings - Fork 46
fix: fix runtime type introspection by moving public API types out of TYPE_CHECKING #499
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
ffdb481 to
5c5df65
Compare
|
@ap-- Hi, could you have a look on this one? |
|
Hi @junjzhang, Thank you for your contribution! Can you provide some more context?
|
Hi,
import tyro
from upath import UPath
from pydantic import BaseModel
class Config(BaseModel):
path: UPath = UPath("s3://bucket/output/")
config = tyro.cli(Config)
from typing import Literal
import upath.implementations.cloud as cloud_mod
from upath._chain import FSSpecChainParser
from typing_extensions import Unpack
from upath.types.storage_options import (
HfStorageOptions,
S3StorageOptions,
GCSStorageOptions,
AzureStorageOptions,
)
cloud_mod.Literal = Literal
cloud_mod.FSSpecChainParser = FSSpecChainParser
cloud_mod.Unpack = Unpack
cloud_mod.S3StorageOptions = S3StorageOptions
cloud_mod.GCSStorageOptions = GCSStorageOptions
cloud_mod.AzureStorageOptions = AzureStorageOptions
cloud_mod.HfStorageOptions = HfStorageOptions
cloud_mod.Literal = Literal
import tyro
from upath import UPath
from pydantic import BaseModel
class Config(BaseModel):
path: UPath = UPath("s3://bucket/output/")
config = tyro.cli(Config)
# OR you could try to use get_type_hints like in the description |
|
Thank you for the additional context 🙏 Given my quick tests, I don't think your solution solves your problem. Can you show me an example how you would provide a path to your tyro cli? ❯ python tyro_example.py --help
usage: tyro_example.py [-h] [--path.kwargs {fixed}]
╭─ options ───────────────────────────────────────────────╮
│ -h, --help show this help message and exit │
╰─────────────────────────────────────────────────────────╯
╭─ path options ──────────────────────────────────────────╮
│ Default: s3://bucket/output │
│ ────────────────────────────────────── │
│ --path.kwargs {fixed} (fixed to: {}) │
╰─────────────────────────────────────────────────────────╯
❯ python tyro_example.py gcs://bucket/key
╭─ Parsing error ───────────────────────────────╮
│ Unrecognized arguments: gcs://bucket/key │
│ ───────────────────────────────────────────── │
│ For full helptext, run tyro_example.py --help │
╰───────────────────────────────────────────────╯
Also tyro seems to have two bugs/missing-features? It does not seem to understand the Given these limitations in tyro, IMO you'd be much better served with writing a custom constructor: https://brentyi.github.io/tyro/examples/custom_constructors/ Would you be interested in giving that a shot to see if we can then find a better way to make this more convenient with universal-pathlib? |
Oh, that's just a minimal reproduce, my daily use case is : # ******* PATCH *******, remove will cause error
from typing import Literal
import upath.implementations.cloud as cloud_mod
from upath._chain import FSSpecChainParser
from typing_extensions import Unpack
from upath.types.storage_options import (
HfStorageOptions,
S3StorageOptions,
GCSStorageOptions,
AzureStorageOptions,
)
cloud_mod.Literal = Literal
cloud_mod.FSSpecChainParser = FSSpecChainParser
cloud_mod.Unpack = Unpack
cloud_mod.S3StorageOptions = S3StorageOptions
cloud_mod.GCSStorageOptions = GCSStorageOptions
cloud_mod.AzureStorageOptions = AzureStorageOptions
cloud_mod.HfStorageOptions = HfStorageOptions
cloud_mod.Literal = Literal
# ******* PATCH END *******
from typing import Annotated
import tyro
from upath import UPath
from pydantic import BaseModel, PlainValidator, PlainSerializer
def serialize_path(value: UPath | str) -> str:
"""Serialize UPath/str to plain str to avoid dict-like JSON output."""
return str(value)
def validate_path_str(value) -> UPath:
"""Validate and convert to UPath."""
if isinstance(value, str):
return UPath(value)
elif isinstance(value, UPath):
return value
elif hasattr(value, "__fspath__"): # Support PathLike objects
return UPath(value)
else:
# Try to convert to string first
try:
return UPath(str(value))
except Exception as e:
raise ValueError(f"Expected str or path-like object, got {type(value)}") from e
PathStr = Annotated[
str | UPath,
PlainValidator(validate_path_str),
PlainSerializer(serialize_path, return_type=str),
]
class Config(BaseModel):
path: PathStr = UPath("s3://bucket/output/")
config = tyro.cli(Config)And the output should be python dev/test_upath.py --help
usage: test_upath.py [-h] [{path:str,path:u-path}]
╭─ options ───────────────────────────────────────────────────────────────────────╮
│ -h, --help show this help message and exit │
╰─────────────────────────────────────────────────────────────────────────────────╯
╭─ optional subcommands ──────────────────────────────────────────────────────────╮
│ (default: path:str) │
│ ─────────────────────────────────────────────────────────────────────────────── │
│ [{path:str,path:u-path}] │
│ path:str │
│ path:u-path Base class for pathlike paths backed by an fsspec filesystem. │
╰─────────────────────────────────────────────────────────────────────────────────╯
python dev/test_upath.py path:str gcs://bucket/key # Will be fine |
But I tried use custom_constructor, I guess it would be a better walk-around # # ******* PATCH *******, remove will cause error
# from typing import Literal
# import upath.implementations.cloud as cloud_mod
# from upath._chain import FSSpecChainParser
# from typing_extensions import Unpack
# from upath.types.storage_options import (
# HfStorageOptions,
# S3StorageOptions,
# GCSStorageOptions,
# AzureStorageOptions,
# )
# cloud_mod.Literal = Literal
# cloud_mod.FSSpecChainParser = FSSpecChainParser
# cloud_mod.Unpack = Unpack
# cloud_mod.S3StorageOptions = S3StorageOptions
# cloud_mod.GCSStorageOptions = GCSStorageOptions
# cloud_mod.AzureStorageOptions = AzureStorageOptions
# cloud_mod.HfStorageOptions = HfStorageOptions
# cloud_mod.Literal = Literal
# # ******* PATCH END *******
from typing import Annotated
import tyro
from upath import UPath
from pydantic import BaseModel
def construct_UPath(
path_str: str
) -> UPath:
"""A custom constructor for UPath."""
return UPath(path_str)
class Config(BaseModel):
path: Annotated[UPath, tyro.conf.arg(constructor=construct_UPath)] = UPath("s3://bucket/output/")
config = tyro.cli(Config)But is this case with built-in method from typing import get_type_hints
from upath.implementations.cloud import S3Path
hints = get_type_hints(S3Path.__init__)
# will raise error |
|
Awesome 🎉 Well with this example it seems you got your use case working correctly: # cli_tyro.py
from typing import Annotated
import tyro
from upath import UPath
from pydantic import BaseModel
def construct_UPath(path_str: str) -> UPath:
"""A custom constructor for UPath."""
return UPath(path_str)
class Config(BaseModel):
path: Annotated[UPath, tyro.conf.arg(constructor=construct_UPath)] = UPath("s3://bucket/output/")
config = tyro.cli(Config)
print(repr(config.path))universal_pathlib on cut-release-0.3.7 [$!?] via 🐍 v3.13.7 (universal-pathlib) via 🅒 base
❯ python cli_tyro.py --path.path-str 'gcs://bucket/key'
GCSPath('bucket/key', protocol='gcs')
universal_pathlib on cut-release-0.3.7 [$!?] via 🐍 v3.13.7 (universal-pathlib) via 🅒 base
❯ python cli_tyro.py --path.path-str 's3://bucket/key'
S3Path('bucket/key', protocol='s3')
universal_pathlib on cut-release-0.3.7 [$!?] via 🐍 v3.13.7 (universal-pathlib) via 🅒 base
❯ python cli_tyro.py --path.path-str 'http://bucket.com/path'
HTTPPath('http://bucket.com/path', protocol='http')
The problem with your fix is, that tyro seems to do something strange with |
Great!, let me change it into a draft pr and put a issue on tyro |
Problem
Classes like
S3Path,GCSPathetc. have type annotations in their__init__that reference types only imported under
if TYPE_CHECKING:. This breaks runtimetype introspection tools that call
typing.get_type_hints(), such as:here is a quick reproduce:
Root Cause
When
from __future__ import annotationsis used, all annotations become strings.get_type_hints()evaluates these strings in the module's namespace. Types underTYPE_CHECKINGdon't exist at runtime, causingNameError.Solution
Move types used in public API signatures out of
TYPE_CHECKINGblock and importthem unconditionally. These are all lightweight imports with no performance impact:
Literal- typing primitiveFSSpecChainParser- already imported elsewhereUnpack,Self- typing utilities