Split Result type into Ok and Err class

[Emerentius]

and make Result[T, E] a type alias for typing.Union[Ok[T], Err[E]].
This allows type safe access to the result members via isinstance
checks.

Implementation of #17 for discussion.

This is of course a big breaking change. I've switched the order of the T and E parameters to be in line with Rust. I haven't added the Ok() fallback for Ok[bool], yet. I'm a bit confused on why that was added, especially with a bool member but it can of course be added again.

The big upside of this way is type safety. A downside is that you can no longer do checks like isinstance(res, Result) because Result is no longer a real type. Consequently, if you want to have a Union[Result[T, E], U], it will be less ergonomic to go back to Result[T, E].

[dbrgn]

Nice, thanks!

Regarding Ok(), I think that was added to simplify the use case where a function without return value can fail.

However, I think that in the next version we should try to avoid magic, and be explicit in all decisions. Removing the default value (True right now) might be part of that.

A downside is that you can no longer do checks like isinstance(res, Result) because Result is no longer a real type.

Could we maybe add an empty BaseResult type or something like that, which the two result variants extend?

[dbrgn]

Btw, we should probably add a note to the README that this library is best enjoyed together with mypy and type annotations.

[Emerentius]

Could we maybe add an empty BaseResult type or something like that, which the two result variants extend?

Yes, but you can already use a tuple to check for multiple types at once. isinstance(res, (Ok, Err)) would be the replacement for isinstance(res, Result).
A baseclass couldn't replace the Result alias because both Ok and Err are generic over one type and Result is generic over two. As far as I can see, it would be useful solely for isinstance, but could confuse people elsewhere. Someone might think you were supposed to use it for type annotations.
We could export an alias for (Ok, Err), like idk, Res or Result_. That may be useful, although it could also cause some confusion. If you tried to use it for type annotations it would throw an error at least. At least if and until PEP 585 is accepted.

[Emerentius]

Ok and Err should also probably be covariant so that Ok[Cat] is a subtype of Ok[Animal] if Cat is a subtype of Animal. I haven't found out how you can do that, yet.

As far as I can see, it isn't possible currently due to the restrictive way mypy treats variance. If you have a class covariant over T, you must never use T as an argument. python/mypy#734 is just about forbidding this exact use case.

[Emerentius]

I believe the error is from an outdated mypy. I couldn't install a development environment from requirements-dev.txt, so I set up a fresh poetry project and used that to setup my dev environment with up-to-date versions. With mypy 0.730 there are no errors.

[coveralls]

Coverage remained the same at 100.0% when pulling 6e72211 on Emerentius:type_safe_result into 0778597 on dbrgn:master.

[Emerentius]

I think the most faithful way to model sum types is to create classes for each enum member like I've done here and then to wrap them into a class with just one attribute member. The wrapper class can then contain all the methods like unwrap_or, you can use isinstance(var, Result) and the equivalent of matching through isinstance(var.member, Ok).

This structure also allows nested enums. If you just had a union type alias for the enum Union[Every, Single, Member] (like in this PR currently), then nested enum types would always be flattened into one mega-enum: Union[Member, Union[SubEnumMember, SubEnumMember2]] == Union[Member, SubEnumMember, SubEnumMember2]

Example (using dataclass for brevity):

@dataclass
class Ok(Generic[T]):
    value: Final[T]

@dataclass
class Err(Generic[E]):
    value: Final[E]

@dataclass
class Result(Generic[T, E]):
    member: Union[Ok[T], Err[E]]

    def unwrap_or(self, value: T) -> T:
        if isinstance(self.member, Ok):
            return self.member.value
        else:
            return value

Downside: It's even more verbose

[dbrgn]

Replaced by #27.

Thanks @Emerentius for getting this started. If possible, I'll try to retain your co-ownership of the initial commit when merging the other PR 🙂