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

Python: typematching against an Interface leads to incorrect pattern matches #3972

Open
kMutagene opened this issue Dec 9, 2024 · 6 comments
Labels
python Python

Comments

@kMutagene
Copy link

kMutagene commented Dec 9, 2024

Description

It seems like, when typematching against an interface, incorrect match result code is generated.

Repro code

See REPL

or transpile this code:

type IInterface =
    abstract member LOL : int

let typeMatchSomeBoxedObject (o:obj) =
    match o with
    | :? int -> 1
    | :? IInterface -> 2
    | _ -> 3

let d = (typeMatchSomeBoxedObject "lol")

printfn "%A" d

the resulting python code:

from abc import abstractmethod
from typing import (Protocol, Any)
from fable_modules.fable_library.decimal_ import from_parts
from fable_modules.fable_library.string_ import (to_console, printf)
from fable_modules.fable_library.types import uint8

class IInterface(Protocol):
    @property
    @abstractmethod
    def LOL(self) -> int:
        ...

def type_match_some_boxed_object(o: Any=None) -> int:
    if str(type(o)) == "<class \'int\'>":
        return 1
    else: 
        return 2

d: int = type_match_some_boxed_object("lol")

to_console(printf("%A"))(d)

Expected and actual results

Expected: A type that does not implement the interface does not match the second match case and therefore 3 is returned via wildcard match case. This is what happens when executing the F# code

Actual: 2 is returned, as the wildcard match case is not generated

Related information

  • Fable version: 4.24.0
@MangelMaxime
Copy link
Member

As a note, Fable for JavaScript is not able to type check against an interface and generates

export function typeMatchSomeBoxedObject(o) {
    if (typeof o === "number") {
        return 1;
    }
    else {
        return 3;
    }
}

but Fable JavaScript generates a warning when type checking against an interface to inform the user.


Going back to the Python target, looking at the code I suppose we should be able to type check against IInterface because the interface does generate an actual Python type.

Looking online, I see that we could use isinstance or type to do that. What should be used here? My supposition is isinstace because it will also check subclasses.

@MangelMaxime MangelMaxime added the python Python label Dec 13, 2024
@kMutagene
Copy link
Author

JFC Here is a workaround we are using atm to solve this for both python and JS in https://github.com/CSBiology/DynamicObj

Our usecase is that we want to call System.ICloneable.Clone() if that interface is implemented by a boxed object.
So we create these native emits:

See also this REPL

JS:

[<Emit("""$0["System.ICloneable.Clone"] != undefined && (typeof $0["System.ICloneable.Clone"]) === 'function'""")>]
let implementsICloneable (o:obj) : bool =
    jsNative

[<Emit("""$0["System.ICloneable.Clone"]()""")>]
let cloneICloneable (o:obj) : obj =
    jsNative

Python:

[<Emit("""hasattr($0, 'System_ICloneable_Clone') and callable($0.System_ICloneable_Clone)""")>]
let implementsICloneable (o:obj) : bool =
     nativeOnly

[<Emit("""$0.System_ICloneable_Clone()""")>]
let cloneICloneable (o:obj) : obj =
    nativeOnly

And the match case can then be conditionally transpiled:

let f (o:obj) =
    match o with

    #if FABLE_COMPILER_JAVASCRIPT || FABLE_COMPILER_TYPESCRIPT
    | o when FableJs.implementsICloneable o -> FableJs.cloneICloneable o
    #endif

    #if FABLE_COMPILER_PYTHON
    | o when FablePy.implementsICloneable o -> FablePy.cloneICloneable o
    #endif

    #if !FABLE_COMPILER
    | :? System.ICloneable as clonable -> clonable.Clone()
    #endif
    
    | _ -> failwith "nah"

@MangelMaxime
Copy link
Member

I was able to have a look at this issue and it seems like we could make Python target support type check against interfaces by generating:

@runtime_checkable
class IInterface(Protocol):
	pass

However, when using @runtim_checkable and isinstance Python will use structural typing:

A Protocol is matched based on its required members (methods, properties, etc.), not based on its name or inheritance.

Because of that:

@runtime_checkable
class IInterface(Protocol):
    @property
    @abstractmethod
    def LOL(self) -> int:
        ...

@runtime_checkable
class AnotherSuperbInterface(Protocol):
    @property
    @abstractmethod
    def LOL(self) -> int:
        ...

are both equivalent.

structural typing is actually similar to what you are doing @kMutagene in your solution as in you are looking if the object has the shape that you are looking for.


I see several solutions:

  1. Behave like in JavaScript and don't allow type testing against interfaces.

  2. Be ok with interface of similar shapes to match the same test (this is actually the standard Python behavior from what i understand)

  3. Look into making the interface have a different shape for example, we could generate a "fake" unique property on the interface

    @runtime_checkable
    class IInterface(Protocol):
        @property
        @abstractmethod
        def LOL(self) -> int:
            ...
    
        @property
        def unique_interface_760868ae_04cd_45ba_b882_2560d666312b(self) -> None:
            pass

My personally opinion is that 3 is the best solution but 2 is also acceptable. I think that allowing interface type checking to work similarly as in Python offers new possibility to code compared to not allowing it at all.

If we go with solution 2, we could like for JavaScript generate a warning explaining that type checking for interface is done via structural typing, blabla...

@kMutagene
Copy link
Author

Hey @MangelMaxime thanks for the update. It is great that this can be implemented to work out-of-the box in python, and i'd agree that solution 3 sounds best, the only issue i'd have with that is that the unique props could clutter types implementing many interfaces real fast, and look weird on intellisense/tooltips in native python.

However, i did not recognize that JS simply does not create any match case at all, since our tests failed only for python i was assuming that this is a python only issue. Sadly this means for us that we still have to resort to a custom emit-based implementation, since the package we are creating is intended to be usable in native code in .NET, JS and py, even when this issue would be fixed for python.

@MangelMaxime
Copy link
Member

MangelMaxime commented Dec 19, 2024

I spend some time this afternoon trying to implements solution 3 and manage to make it work somewhat with others issues popping up 😅

Pure interface testing works but when trying to type test using interface against class it was failing. Indeed, Fable (Python) doesn't make the class inherit from the interface:

let interfaces, stmts =
// We only use a few interfaces as base classes. The rest is handled as Python protocols (PEP 544) to avoid a massive
// inheritance tree that will prevent Python of finding a consistent method resolution order.
let allowedInterfaces = [ "IDisposable" ]
ent.AllInterfaces
|> List.ofSeq
|> List.filter (fun int ->
let name = Helpers.removeNamespace (int.Entity.FullName)
allowedInterfaces |> List.contains name
)

I tried to disable this and now the specific use case from this issue works, but like implied in the comment linked above the Python to resolve the code:

Cannot create a consistent method resolution order (MRO) for bases IComparable, IEquatable, IStructuralEquatable, IComparable_1, IStructuralComparable, Generic

However, i did not recognize that JS simply does not create any match case at all, since our tests failed only for python i was assuming that this is a python only issue.

I think for now, I am going to try to make Python emit the same code as JavaScript.

And in the future, we can try to revisit interface Type checking to make them work fully. It seems like there are some potential to make them work but I currently don't know well enough Python and the generated Python code by Fable.

My experimentation can be found in https://github.com/fable-compiler/Fable/tree/experimentation/python/interface_type_checking

cc @dbrattli In case, you have some idea or information to share regarding the current issue.

@MangelMaxime
Copy link
Member

@kMutagene If you see this warning in Fable output:

warning FABLE: Cannot type test (evals to false): interfaces

it means that you are type testing against an interface and that the test will always fail at runtime equal to false.

I don't know why this is a warning only and not an error / compilation failure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
python Python
Projects
None yet
Development

No branches or pull requests

2 participants