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] Should obj () generate an empty dict ? #3598

Open
MangelMaxime opened this issue Nov 19, 2023 · 12 comments
Open

[Python] Should obj () generate an empty dict ? #3598

MangelMaxime opened this issue Nov 19, 2023 · 12 comments
Labels
python Python

Comments

@MangelMaxime
Copy link
Member

Description

Currently let a = obj () generates b : Any = None but this cause problem if later user try to set a property on that object.

open Fable.Core.PyInterop

let b : obj = obj ()
let prop = "name"
b?(prop) <- "maxime"

This generates the error:

AttributeError: 'NoneType' object does not support item assignment

But changing the emit code to {} works:

open Fable.Core.PyInterop

let b : obj = emitPyExpr () "{}"
let prop = "name"
b?(prop) <- "maxime"

It generates

b: Any = {}

prop: str = "name"

b[prop] = "maxime"

Related information

  • Fable version: 4.5.0
  • Operating system: OSX
@MangelMaxime MangelMaxime added the python Python label Nov 20, 2023
@dbrattli
Copy link
Collaborator

Sounds like a good idea! Alt least it should not be None. Not sure why it generated None to begin with.

However, the problem with dict() i.e {} is that you cannot use member access (e.g b.prop) and have to use item access (e.g p[prop]). I don't know a simple way of telling a dict from an object without doing run-time type checking e.g isinstance.The simplest object e.g (object()) do not support item assignment and you cannot use member assignment either since it has no members.

The problem with the above code is that in release mode the property will be inlined and we will use attribute access which will fail with Python. I need to look into the old logic to see if it still makes sense and what we can do with it (if any).

@dbrattli
Copy link
Collaborator

Could you tell more about the use-case? Can you have special code for Python or is it similar code that needs to be used for Python and JS? PS: The .NET Dictionary will translate to dict i.e {}.

@dbrattli
Copy link
Collaborator

Candidates for obj() can be:

  1. dict(), uses subscript access
  2. object(), no subscript or attribute access
  3. Class AttrDict: pass, allows attribute access, but not subscript access

@MangelMaxime
Copy link
Member Author

The usage that I am working on Thoth.Json (library agnostic version).

This means that I am creating a core library which has some helpers contract that can then be used implemented by the different runtimes like JavaScript, Python, Newtonsoft, etc.

For object, my current API implementation looks like this:

let rec toJsonValue (helpers: IEncoderHelpers<'JsonValue>) (json: Json) =
    match json with
    | Json.Object values ->
        let o = helpers.createEmptyObject ()

        values
        |> Seq.iter (fun (k, v) ->
            helpers.setPropertyOnObject (o, k, toJsonValue helpers v)
        )

        o

Where for JavaScript I have this implementation

[<RequireQualifiedAccess>]
module Encode =

    let helpers =
        { new IEncoderHelpers<obj> with
            member _.createEmptyObject() = obj ()

            member _.setPropertyOnObject(o: obj, key: string, value: obj) =
                o?(key) <- value
         }

For newtonsoft:

[<RequireQualifiedAccess>]
module Encode =

    let helpers =
        { new IEncoderHelpers<JToken> with
            member _.createEmptyObject() = JObject()

            member _.setPropertyOnObject
                (
                    o: JToken,
                    key: string,
                    value: JToken
                )
                =
                o[key] <- value
        }

And for Python I do:

[<RequireQualifiedAccess>]
module Encode =

    let helpers =
        { new IEncoderHelpers<obj> with
            member _.createEmptyObject() = emitPyExpr () "{}"

            member _.setPropertyOnObject(o: obj, key: string, value: obj) =
                o?(key) <- value
        }

For now for Python, I am using emitPyExpr because I discovered the bug that we are currently discussing.

Note: The object created this way will then be pass to JSON.dumps to generate the JSON.

However, the problem with dict() i.e {} is that you cannot use member access (e.g b.prop) and have to use item access (e.g p[prop]). I don't know a simple way of telling a dict from an object without doing run-time type checking e.g isinstance.The simplest object e.g (object()) do not support item assignment and you cannot use member assignment either since it has no members.

This is indeed something I discovered yesterday. And I was wondering if in Python p.prop and p[prop] where equivalent or not? And if we should not just generate p[prop] all the time if they are equivalent.

Also when reading about how to create an empty object in Python, some people said that dict() was slower that {} because they said that dict() was just a function that return {}.

@dbrattli
Copy link
Collaborator

I think you are right about {} vs dict(). But one thing with the latter you can give type arguments e.g dict[str, str]() which can be nice sometimes. Anyways, would Systems.Collections.Generic.Dictionary work for you? Then you would not have to use interop.

@MangelMaxime
Copy link
Member Author

Hum, in my case I don't think I can use Systems.Collections.Generic.Dictionary because of the API contract I have.

type IEncoderHelpers<'JsonValue> =
    abstract createEmptyObject: unit -> 'Object
    abstract setPropertyOnObject: 'JsonValue * string * 'JsonValue -> unit
    abstract encodeString: string -> 'JsonValue

let rec toJsonValue (helpers: IEncoderHelpers<'JsonValue>) (json: Json) : 'JsonValue =
    // ...

I tried to add a new 'JObject generic type but in the end the compiler tells me that there is a conflict at some point.

I could probably make it works by using some unbox/box magic but I believe the current situation using interop is better as makes the written code easier to read than box/unbox and keep the generated output clean too.

@dbrattli
Copy link
Collaborator

The problem I have with the Fable AST is that it has a Set expr, but I don't know how I can tell if it should be subscript/index (e.g a["b"]) or attribute/property based (e.g a.b)

@MangelMaxime
Copy link
Member Author

Is there an important difference between them?

What if we always used subscript/index notation when using dynamic typing?

I am really naive I know ^^

@dbrattli
Copy link
Collaborator

@ncave I'm unsure about the Fable AST if there is a way to tell if something should be subscripted or use attribute access for SetKind and GetKind. Do you have this problem in Rust? That some gets should be a.b while others should be a[b]. How do you handle it?

@ncave
Copy link
Collaborator

ncave commented Nov 26, 2023

@dbrattli Usually indexers are compiled as method calls, but perhaps you mean something else. Can you paste a small F# example of both?

@MangelMaxime
Copy link
Member Author

@dbrattli Can this is be closed or do you still need to discuss some of the raised issues from the discussion?

@dbrattli
Copy link
Collaborator

@MangelMaxime I don't think the issue is fixed yet. The code will now generate an empty dict for obj (), but setters and getters still do not work as expected. I'm planning a larger refactor to try and fix this issue, but haven't had the time yet. Perhaps in the holidays ahead.

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

3 participants