Fable icon indicating copy to clipboard operation
Fable copied to clipboard

python: anonymous record creation generates camel case dict fields

Open joprice opened this issue 1 year ago • 4 comments

Description

When using the python backend and making use of an anonymous record, the accessors use snake case, while the construction code has camel cased fields, resulting in a KeyError when called:

Repro code

type data = {| fromId: int |}

// this creates a dict with camel case field "fromId"
let makeData() = {| fromId = 1 |}

let y (d: data) = 
  d.fromId
from typing import Any

def make_data(__unit: None=None) -> dict[str, Any]:
    return {
        "fromId": 1
    }


def y(d: dict[str, Any]) -> int:
    return d["from_id"]

https://fable.io/repl/#?code=FAFwngDgpgBAJgQxAmBeGBvAPjAZgJwHsBbASTgC4YBLAOxBiwF9hgB6NmEAC2oGcYAY3xQkUASjjVBDAO7UeQhMSgAbJX1i5qauDABEBEuX3BVUBsQQBrKABEkCABQBKNJhxGye9AEZGLGYWMGAwTpTwjm7owDDwAHRe5EA&html=Q&css=Q

Related information

  • Fable version: 4.19.3
  • Operating system: osx

joprice avatar Jul 22 '24 16:07 joprice

Something else to note: when the field starts with a capital, it is left unchanged:

type data = {| FromId: int |}

// this creates a dict with camel case field "fromId"
let makeData() = {| FromId = 1 |}

let y (d: data) = 
  d.FromId
from typing import Any

def make_data(__unit: None=None) -> dict[str, Any]:
    return {
        "FromId": 1
    }


def y(d: dict[str, Any]) -> int:
    return d["FromId"]

joprice avatar Jul 22 '24 16:07 joprice

A similar problem happens when using Record.

type Test =
    {
        FirstName : string  
        lastName : string
    }

let test =
    {
        FirstName = ""
        lastName = ""
    }

test.FirstName
test.lastName
from __future__ import annotations
from dataclasses import dataclass
from fable_library_js.reflection import (TypeInfo, string_type, record_type)
from fable_library_js.types import Record

def _expr0() -> TypeInfo:
    return record_type("Test.Test", [], Test, lambda: [("FirstName", string_type), ("last_name", string_type)])


@dataclass(eq = False, repr = False, slots = True)
class Test(Record):
    FirstName: str
    last_name: str

Test_reflection = _expr0

test: Test = Test("", "")

test.FirstName

test.last_name

I suspect that the convention in Python is to use snake_case? @dbrattli

MangelMaxime avatar Jul 22 '24 18:07 MangelMaxime

As far as I can understand, the record one seems to be working, in the sense that the class contains the same fields as the accessor used at the call-site: FirstName:str vs test.FirstName. Whereas with the anonymous one, the two do not line up: the creation side uses camel case {"fromId": 1 } and the accessor uses snake case d["from_id"].

So it seems there's a convention in the python backend of converting to snake case automatically, but it's not implemented correctly for anonymous records.

joprice avatar Jul 22 '24 18:07 joprice

Yes, but it also seems like the snake_case conversion is only if you use camelCase.

I don't know if this should also be applied to properties that use PascalCase.

However, if we do so there is a need to mangle the name of the property because FirstName and firstName would both be translated to first_name. Perhaps, this is the reason why only camelCase are transformed.

MangelMaxime avatar Jul 22 '24 19:07 MangelMaxime

I am having the same issue (#4129), and would be great if there is a way to disable the case conversion. The behaviour is a bit too implicit at the moment.

x-EricH-x avatar Jun 09 '25 20:06 x-EricH-x

For Python code should be for formatted and styled according to PEP-8. This means that only types are CapitalizedWords, and almost anything else is lower_case_with_underscores. This is important for Python interop but I see it can be confusing. Currently anything starting with lower-case will be snake_cased. Perhaps we could use attributes to make this more explicit e.g [<CamelCase>], [<SnakeCase>] and [<PascalCase>] on the record. If no attribute, then the member/field/attribute will be encoded as-is (i.e no translation). What do you all think? @MangelMaxime

dbrattli avatar Jun 11 '25 19:06 dbrattli

I think there are 2 issues here:

  1. The conversion is inconsistent at the moment and will create code that trigger KeyError
  2. Fable docs describe anonymous records as a simple way to work with dict (https://fable.io/docs/python/features.html#anonymous-records) With the implicit conversion however, it is creating frictions when the use case expect the keys to remain the same, e.g. when using them as json payload. (The casing guideline in PEP-8 is for Python identifiers, but for dict keys I think we can have more freedom/controls.)

For (1) I would say it is a bug should be fixed. For (2) Yes, would be great if the conversion can be controlled by attributes.

x-EricH-x avatar Jun 11 '25 22:06 x-EricH-x

Yes, I for anonymous records I agree that we should not do any conversion of the names. Both this and the KeyError is easy to fix. For Records I'm still unsure since it transpiles to a Python class, and for Python classes we would want PEP-8 semantics, but again it would be great if we could override this with e.g attributes.

dbrattli avatar Jun 14 '25 10:06 dbrattli

Perhaps we could use attributes to make this more explicit e.g [<CamelCase>], [<SnakeCase>] and [<PascalCase>] on the record. If no attribute, then the member/field/attribute will be encoded as-is (i.e no translation). What do you all think? @MangelMaxime

I am not a fan of this approach because it means people will need to add a lot of attributes in order to generate "standard" Python code.

I am in favor of having the conversion enforced automatically for the user like done currently with good default. If in Python, everything or almost should use snake_case then let's do that. That way the behavior is easy to understand for the user.

type User =
    {
        FirstName : string
        lastName : string
        LastName : string // Casing different on purpose
    }

should generates something like (if I understood correctly properties/field in PEP-8 should use snake_case)

@dataclass(eq = False, repr = False, slots = True)
class User(Record):
    first_name: str
    last_name@1: str
    last_name@2: str

@1 and @2 are pseudo code but we should use the currently mangled mechanism of Fable to not re-invent the wheel

If for some reason, this is not good for all cases then we could consider supporting CompiledName attributes which is standard in F#, but currently Fable don't respect it for Record properties (at least for JavaScript).

Regarding the original problem reported in this issue, I think we should fix the call site to use the same convention at the definition site.

MangelMaxime avatar Jun 16 '25 13:06 MangelMaxime

One problem I see with any translation of field names for Records is reflection info. It's quite hard to generate code that preserves .NET semantics in Python and we easily end up with code that either fails in F# tests, or in the Python tests:

Passes in Python but fails for .NET:

[xUnit.net 00:00:00.40]       Expected: map [(Age, 12); (Firstname, Maxime); (last_name, Mangel)]
[xUnit.net 00:00:00.40]       Actual:   map [(Age, 12); (Firstname, Maxime); (lastName, Mangel)]

Passes for .NET but fails in Python:

actual = 'map [(Age, 12); (Firstname, Maxime); (last_name, Mangel)]', 
expected = 'map [(Age, 12); (Firstname, Maxime); (lastName, Mangel)]',

Any suggestions for this? Is it possible to fix, or should we just preserve casing for Records?

PS: This was the test that gave me problems: https://github.com/fable-compiler/Fable/blob/main/tests/Python/TestReflection.fs#L253

dbrattli avatar Jun 16 '25 22:06 dbrattli

If I understand the issue is that let expected = "map [(Age, 12); (Firstname, Maxime)]" is different between .NET and Python runtime if we change the property name?

If the reflection API works, then we could use a compiler directive for the tests. This is something we are already doing for ToString or Date formatting for example.

MangelMaxime avatar Jun 17 '25 14:06 MangelMaxime

Aha, of course, that would solve the problem. Thank you so much for unblocking me on this one @MangelMaxime 😊

The other crux I ran into btw was type testing such as | :? R3 as x -> compare this.Bar x.Bar since optimized (Release mode) code erases the type of of x to Any for the then case. So I had to figure out a way for if-then-else expressions to pass the type from the guard case back to the then case to know that x is a Record in order to apply the right casing. Hope there's not many more such edge-cases 🙈

<CustomEquality; CustomComparison>]
type R3 =
    { Bar: string }
    interface System.IComparable with
        member this.CompareTo(x) =
            match x with
            | :? R3 as x -> compare this.Bar x.Bar // <- this one
            | _ -> -1
    override this.GetHashCode() = hash this.Bar
    override this.Equals(x) =
        match x with
        | :? R3 as x -> this.Bar = x.Bar // <- and this one
        | _ -> false

dbrattli avatar Jun 17 '25 18:06 dbrattli