Skip to content
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

Use AnyPath instead of to_anypath as constructor by fixing type issues #146

Open
pjbull opened this issue May 29, 2021 · 4 comments
Open
Labels
bug Something isn't working

Comments

@pjbull
Copy link
Member

pjbull commented May 29, 2021

When implementing #142 I encountered a tricky type annotation scenario and used a workaround.

The AnyPath constructor instead of to_anypath in the code referenced below runs fine, but mypy complains. We should determine the right type annotations to use AnyPath(p) instead of to_anypath.


I couldn't use the AnyPath constructor

For some reason, this was the only way I could make mypy happy after the call here. Maybe you've got a nice solution:

# handle string version of cloud paths + local paths
if isinstance(destination, (str, os.PathLike)):
destination = anypath.to_anypath(destination)


If I change that line to anypath.AnyPath(destination), I see these errors even if I # type: ignore that line.

mypy cloudpathlib
cloudpathlib/cloudpath.py:699: error: Item "str" of "Union[str, _PathLike[Any], CloudPath]" has no attribute "exists"
cloudpathlib/cloudpath.py:699: error: Item "_PathLike[Any]" of "Union[str, _PathLike[Any], CloudPath]" has no attribute "exists"
cloudpathlib/cloudpath.py:699: error: Item "str" of "Union[str, _PathLike[Any], CloudPath]" has no attribute "is_file"
cloudpathlib/cloudpath.py:699: error: Item "_PathLike[Any]" of "Union[str, _PathLike[Any], CloudPath]" has no attribute "is_file"
cloudpathlib/cloudpath.py:704: error: Item "str" of "Union[str, _PathLike[Any], CloudPath]" has no attribute "mkdir"
cloudpathlib/cloudpath.py:704: error: Item "_PathLike[Any]" of "Union[str, _PathLike[Any], CloudPath]" has no attribute "mkdir"
cloudpathlib/cloudpath.py:716: error: Incompatible return value type (got "Union[str, _PathLike[Any], CloudPath]", expected "Union[Path, CloudPath]")
Found 7 errors in 1 file (checked 24 source files)

Originally posted by @pjbull in #142 (comment)

@jayqi
Copy link
Member

jayqi commented Feb 9, 2022

I tried a new idea for AnyPath's implementation to address type-checking issues, and had some successes, but unfortunately I don't think it will totally work.

The idea I had was: Python 3.8 introduced the typing.Protocol special base class (and it's also available as a backport). Protocol lets you do structural subtyping / static duck typing, which is basically formalizing duck-typing in a way that static type-checkers can use that information. It also supports runtime type-checking for isinstance and issubclass.

So what I tried was getting the intersection of the CloudPath and Path interfaces and making an AnyPath protocol class. Much of this worked as advertised: isinstance and mypy were both happy when checking whether CloudPath or Path are subtypes of AnyPath.

However, I still ran into some limitations that I think means this doesn't sufficiently solve our problems:

  • issubclass doesn't work: TypeError: Protocols with non-method members don't support issubclass(). This seems to just be a hard limitation of Protocol. We have properties so I don't think there is any way around this.
  • mypy still won't be happy when you try to use AnyPath as a constructor: error: Cannot instantiate protocol class "AnyPath"
  • The official way to support custom type validation in Pydantic requires adding a __get_validators__ method. Unfortunately, this breaks the protocol from working since now it's requiring that subtypes have that method too. (Mainly a problem with Path, since we also have __get_validators__ on CloudPath.) isinstance stops working, and mypy has the following complaint:
cloudpathlib/anypath.py:90: note: "Path" is missing following "AnyPath" protocol member:
cloudpathlib/anypath.py:90: note:     __get_validators__

We could maybe work around the Pydantic thing by monkeypatching Pydantic's internals to include our custom validator without adding a method to our AnyPath protocol class. We could also open an issue on Pydantic and asking for official support for registering a validator as another way of supporting custom types.

Code from my experimentation on this branch.

Here are also some minimal examples that demonstrate the basic principles:

Protocol with constructor dispatch

from typing import Protocol, runtime_checkable


@runtime_checkable
class Base(Protocol):

    def __new__(cls, val: int):
        if val >= 0:
            return ChildA(val)
        else:
            return ChildB(val)

    def method(self):
        pass


class ChildA:

    def __init__(self, val: int):
        self.val = val

    def method(self):
        pass


class ChildB:

    def __init__(self, val: int):
        self.val = val

    def method(self):
        pass


## Construction dispatch works

Base(4)
#> <ChildA object at 0x7fc36281ce50>
Base(-9)
#> <ChildB object at 0x7fc36281c7c0>


## Subtype-checking works for isinstance
 
isinstance(ChildA(10), Base)
#> True
isinstance(ChildB(-5), Base)
#> True

Protocol with dispatch and Pydantic

from typing import Protocol, runtime_checkable
from pydantic import BaseModel


def base_validator(val):
    return Base(val)


@runtime_checkable
class Base(Protocol):

    def __new__(cls, val: int):
        if val >= 0:
            return ChildA(val)
        else:
            return ChildB(val)

    @classmethod
    def __get_validators__(cls):
        yield base_validator

    def method(self):
        pass


class ChildA:

    def __init__(self, val: int):
        self.val = val

    def method(self):
        pass


class ChildB:

    def __init__(self, val: int):
        self.val = val

    def method(self):
        pass


## Construction dispatch works

Base(4)
#> <ChildA object at 0x7fc36286a1f0>
Base(-9)
#> <ChildB object at 0x7fc36286a970>


## Pydantic dispatch works

class MyModel(BaseModel):
    base: Base

MyModel(base=9)
#> MyModel(base=<ChildA object at 0x7fc36281c700>)
MyModel(base=-4)
#> MyModel(base=<ChildB object at 0x7fc36281c310>)

## Type-checking fails
        
isinstance(ChildA(10), Base)
#> False
isinstance(ChildB(-5), Base)
#> False

Created at 2022-02-08 23:09:15 EST by reprexlite v0.4.2

@M0dEx
Copy link

M0dEx commented Sep 5, 2024

Maybe something like pathlib_abc.PathBase might be helpful?

It would possibly break isinstance(path, Path) checks but would provide much-needed type-safety.

@jayqi
Copy link
Member

jayqi commented Sep 5, 2024

There's also some discussion about PathBase in #347.

@M0dEx
Copy link

M0dEx commented Sep 9, 2024

It seems this arcane spell fixes most of the typing issues for us:

SfAnyPath: TypeAlias = cast(  # type: ignore
    Annotated[
        Callable[[str], CloudPath | Path], BeforeValidator(AnyPath.validate), PlainSerializer(str)
    ],
    AnyPath,
)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants