fable-compiler / Fable

F# to JavaScript, TypeScript, Python, Rust and Dart Compiler
http://fable.io/
MIT License
2.93k stars 301 forks source link

Python Support for Fable #2339

Closed dbrattli closed 3 years ago

dbrattli commented 3 years ago

Description

This issue is a discussion about Python support for Fable. A POC has been made to show that this is possible. However, quite a lot of work is needed to:

  1. Add decent support for Python
  2. Make Fable more JS agnostic and support multiple target languages.

Use cases:

Things to discuss and decide:

Installing the POC

The source code of the POC currently lives here:

  1. Install the latest Python 3.9 from https://www.python.org or brew install python@3.9 on Mac. Note that python may be available on your system as python, python3 or both.
  2. Clone both repo and switch to the python branch.
  3. Build fable-library for Python: dotnet fsi build.fsx library-py
  4. To run tests: dotnet fsi build.fsx test-py
  5. In Fable, edit QuickTest.fs to some simple F# code. Then run dotnet fsi build.fsx quicktest-py from the top directory.

Now you can edit both Fable and Expression code and the example code will recompile when needed. Note you need an additional save to QuickTest.fs to trigger recompile after modifying Fable code. The python file will be available as quicktest.py. A nice demo is to view both F#, JS, and Python in vscode at the same time. Then you will see how the code is being transformed when you save the F# file:

Screenshot 2021-01-15 at 13 18 09

You can run the generated Python code in the terminal:

$ python3 quicktest.py

Links

A few relevant links:

Feel free to contribute with discussions, comments, ideas, and code šŸ˜

alfonsogarciacaro commented 3 years ago

This is great work @dbrattli! šŸ‘ šŸ‘ šŸ‘ My original idea with Fable was to provide a way to easily compile F# to other languages. For better or worse, Fable has evolved with JS as its main focus so it would require some work to make it more language agnostic but I would be open to that. Some comments to your questions:

PawelStadnicki commented 3 years ago

I love this idea. Especially I'm looking forward to use Python libraries in F#. Ideally we should have the option to consume either pure Python libraries or SciSharp - they tend to have almost exact API as Python (same F# code run in Python or .NET context!)

Such F# code can be compiled with Fable into Python and then used within Notebooks kernels (Jupyter or .NET Interactive) making analytics workflow being real domain-driven facility ( I could consider it all as a "killing feature").

.NET Interactive is very extensible, running Fable inside F# Kernel is very easy ( I have working PoC).

Another thing is that I predict that MS will reveal something similar quiet soon for entire .NET (the prediction is based only on activities and goals .NET Interactive tackle so I may be wrong). Even if they do we still want Fable to have it separately in order to have complete JS/F#/Python arsenal in a functional world. MS approach would be more like a Blazor-flavour ( I guess).

dbrattli commented 3 years ago

Thanks, this is great feedback @alfonsogarciacaro, @PawelStadnicki . What I meant was if we want to let the programmer get the "Python" feeling by exposing more of the Python standard library instead of trying to reimplement .NET (then they should probably just use .NET instead).

I guess we could have the best from both worlds, and have the user decide what to import or not (e.g use datetime from Python or DateTime from .NET). So no need to decide, but just implement what we want the most (on-demand). But I suspect the same problem is relevant for Fable JS. You know that you are implementing a web app, and probably don't expect all of .NET to be available. You would rather have better access to the JS ecosystem.

So no need to decide up-front. Let's roll up the sleeves and get going šŸ˜„ It will be great to be part of Fable and have this issue to ask questions along the way, e.g about transformations etc. There is much to learn.

PS: Expression btw also have a tail-call decorator we can use (both sync and async). Here's an example usage in aioreactive.

jwosty commented 3 years ago

Makes me think of #1601.

I wonder if there's a good way to make Fable powerful enough that these could be done as third-party backend "plugins" (think LLVM but specific for F#). Endless possibilities.

alfonsogarciacaro commented 3 years ago

We could try to make the Babel AST more generic (and typed) so it can be easily transformed/printed into C-like languages. However, if I've learned something while developing Fable is that writing a simple F#-to-another-language compiler is (more or less) easy, but providing a good development experience and integration so the advantages of F# minus the friction with the foreign ecosystem weigh more than developing in the native language is quite hard.

ncave commented 3 years ago

What @alfonsogarciacaro said. Part of that challenge is to migrate more of the Fable BCL implementation to F# so it doesn't have to be rewritten for every language, while maintaining acceptable native performance for the generated code, something which is non-trivial even with JavaScript.

jwosty commented 3 years ago

Interesting. I didn't realize that significant portions of the Fable BCL implementation were written in (presumably) Javascript. I assumed it would be more F# already. I don't doubt there's good reason for this -- why did it turn out to be easier that way?

ncave commented 3 years ago

@jwosty The (not so short) answer is: for performance reasons, and to bridge the difference between F# types vs types natively supported by the browser, and for better integration of the generated code within the JavaScript ecosystem.

Also .NET BCL has a very large API surface, on top of FSharp.Core, so it's hard to maintain this effort without a bigger team (although @alfonsogarciacaro somehow manages to maintain and evolve it mainly by himself, which is amazing and commendable).

I guess what I'm trying to say is, contributions (and ideas) are very welcome and appreciated.

jwosty commented 3 years ago

The (not so short) answer is: for performance reasons, and to bridge the difference between F# types vs types natively supported by the browser, and for better integration of the generated code within the JavaScript ecosystem.

I can see that.

I guess what I'm trying to say is, contributions (and ideas) are very welcome and appreciated.

For sure! I understand how much effort goes into maintaining projects as big and as important as this, and I really appreciate the blood sweat and tears you guys have poured into it. Definitely am keeping my eyes open for places to contribute, as Fable has lately been very very very useful for me and I'd love to give back.

dbrattli commented 3 years ago

I could use a little help. I'm currently using the QuickTest project for development (i.e dotnet fsi build.fsx quicktest). How do I print the Babel AST while developing? (Update: I found ASTViewer).

My current problem is that Python do not support multiline lambda (arrow functions) so the lambda need to be transformed and extracted to a named function and call that function instead. Any tips here would be appreciated. E.g:

let fn args cont  =
    cont args

let b = fn 10 (fun a ->
    a + 1
)

Will need to be transformed to:

let fn args cont  =
    cont args

let cont a =
    a + 1
let b = fn 10 cont

How should I think when replacing one call expression with multiple statements since when transforming the function call with args I need to replace it at a higher level, or use some kind of block statement e.g if true then ... Any ideas?

ncave commented 3 years ago

@dbrattli See also this Printer, which prints some of the properties too.

alfonsogarciacaro commented 3 years ago

I didn't realize that significant portions of the Fable BCL implementation were written in (presumably) Javascript. I assumed it would be more F# already. I'm don't doubt there's good reason for this -- why did it turn out to be easier that way?

Yes, as @ncave said, the main reason was to generate more standard JS. For example, in Funscript all the FSharp.Core replacements were written in F#, but with Fable I wanted to integrate with JS standards when possible (e.g. using JS iterators instead of .NET IEnumerable) which brought many chicken-and-egg problems so it was easier to write it in JS/Typescript at the beginning. At the beginning I also had the idea of publishing fable-library as a separate package that could be used in JS projects, but I quickly discarded it.

In Fable 2 we started an effort to port some of the modules to F#. This has been very beneficial because it increases dogfooding and let us easily reuse the improvements in FSharp.Core (as with maps and sets recently). However, there are still some hacks here and there to work around the chicken-and-egg issues. And let's not talk about the big monster that is the Replacements module :)

Also as @ncave says, maintaining this library is a huge undertake, that's why I'm always reluctant to increase support for BCL to reduce the maintenance surface, but I've been very lucky that there've been multiple great contributions to the library from the very beginning.

@dbrattli About printing the AST, it's been a while I haven't used the ASTViewer tool in Fable repo. Better options are the one pointed by @ncave or the Fantomas visualizer which is what I use recently when I need to inspect the F# AST (just make sure to select Show Typed AST). Unfortunately we don't have specific printers for Fable or Bable AST yet. In the case of Fable AST, as it's just unions and records a simple printfn "%A" works, although the location info is a bit noisy.

About extracting the lambdas. It's possible but requires some work. I guess we can do something similar to imports, that is, collecting them when traversing the AST, and assign them a unique identifier. Later we can just declare the functions at the top of the file. The main challenge will likely be the captured values, I don't remember if the F# AST has information about this, but even if it has it, unfortunately we don't keep it in the AST, so I guess we'll have to find all the idents used in the lambda body which doesn't correspond to arguments (or bindings within the lambda) and convert them to extra arguments of the lambda (and I guess they must go at the beginning to avoid interfering with currying).

dbrattli commented 3 years ago

Thanks for great info @alfonsogarciacaro, my first PoC was just altering Babel.fs / Fable2Babel.fs, BabelPrinter.fs etc. I've now starting from scratch with a separate Python AST i.e Python.fs, PythonPrinter.fs. Then I'll will try to add Babel2Python.fs to transform from Babel AST to Python AST as that may be easier than converting from Fable AST (but also possible with Fable2Python.fs if converting from Babel ends up being difficult). This way I will not touch the existing code base too much, and have my own transformation where I will try to do the lambda rewrite.

alfonsogarciacaro commented 3 years ago

That makes sense. One difficulty is Babel AST is not made of DUs so it's a bit more difficult to traverse it with pattern matching. Maybe #2158 could help?

dbrattli commented 3 years ago

@alfonsogarciacaro I think I managed to fix the rewriting of arrow-functions and function-expressions in a way that I think (hope) might actually work. The idea is that every expression (TransformAsExpr) also returns a list of statements (that needs to be concatenated and passed on all the way up to the last list of statements (last-statement-level). Then all the returned statements (func-def) will be lifted out and up and written in front of other statements. So an arrow- or function-expression will simply return a name-expression, [statement ] where the arrow/function-expression is rewritten to a statement and returned along with the name-expression. This means that:

module QuickTest

let fn args cont  =
    cont args

let b = fn 10 (fun a ->
    printfn "test"
    a + 1
)

Generates the following JS:

import { printf, toConsole } from "./.fable/fable-library.3.1.1/String.js";

export function fn(args, cont) {
    return cont(args);
}

export const b = fn(10, (a) => {
    toConsole(printf("test"));
    return (a + 1) | 0;
});

Which in turn generates the following Python:

from expression.fable.string import (printf, toConsole)

def fn(args, cont):
    return cont(args)

def lifted_5094(a):
    toConsole(printf("test"))
    return (a + 1) | 0

b = fn(10, lifted_5094)

I thought I might have problems handling closures, but in most (all?) cases the closures are also lifted e.g:

module QuickTest

let add(a, b, cont) =
    cont(a + b)

let square(x, cont) =
    cont(x * x)

let sqrt(x, cont) =
    cont(sqrt(x))

let pythagoras(a, b, cont) =
    square(a, (fun aa ->
        printfn "1"
        square(b, (fun bb ->
            printfn "2"
            add(aa, bb, (fun aabb ->
                printfn "3"
                sqrt(aabb, (fun result ->
                    cont(result)
                ))
            ))
        ))
    ))

Will be rewritten to:

from expression.fable.string import (printf, toConsole)

def add(a, b, cont):
    return cont(a + b)

def square(x, cont):
    return cont(x * x)

def sqrt(x, cont):
    return cont(math.sqrt(x))

def pythagoras(a, b, cont):
    def lifted_1569(aa):
        toConsole(printf("1"))
        def lifted_790(bb):
            toConsole(printf("2"))
            def lifted_6359(aabb):
                toConsole(printf("3"))
                return sqrt(aabb, lambda result: cont(result))

            return add(aa, bb, lifted_6359)

        return square(b, lifted_790)

    return square(a, lifted_1569)

Anyways, it's good start šŸ˜„

thinkbeforecoding commented 3 years ago

I did quite the same for PHP, (and it's running CrazyFarmers on BGA) so maybe we could combine the effort here ?

https://github.com/thinkbeforecoding/peeble

dbrattli commented 3 years ago

@ncave Do you btw have any tips about using Fable for interactive usage such as Jupyter notebooks? The problem is that Jupyter will give you a code fragment (cell) and the kernel needs to compile this fragment along with any existing local declarations from previous statements.

Fable isn't optimal for this, but I've created a PoC where I keep an ordered dictionary of previous statements/declarations (let, type, open) and create a valid F# program using them in order along with the last code fragment. Any declarations in the code fragment is removed from the dict before execution and added after. This seems to work, but I'm wondering if there is perhaps a better way?

Source: https://github.com/dbrattli/Fable.Jupyter/blob/main/fable/kernel.py#L85

Then I have a Fable cli running in the background that recompiles the whenever Fable.fs is updated.

dotnet watch -p src/Fable.Cli run -- watch --cwd /Users/dbrattli/Developer/GitHub/Fable.Jupyter --exclude Fable.Core --forcePkgs --python
ncave commented 3 years ago

@dbrattli Sounds reasonable, if fragments are small. For larger ones, perhaps using a separate file for each cell (fragment) can speed things up, as Fable uses some caching to skip unnecessary work on files that didn't change.

Just FYI, if you need more control, you can also use Fable as a library, not just CLI (see fable-standalone, and a usage example).

dbrattli commented 3 years ago

1) A problem I have is that the Fable AST does not have Throw / Raise. So this will be made into an Emit expression that is hard(er) for me to parse. Or at least it seems unnecessary when Babel have a ThrowStatement. Is there a good reason it's not being used, or should I try to fix it? @alfonsogarciacaro

let divide1 x y =
   try
      Some (x / y)
   with
      | :? System.DivideByZeroException -> printfn "Division by zero!"; None

let result1 = divide1 100 0

Generates:

export function divide1(x, y) {
    try {
        return ~(~(x / y));
    }
    catch (matchValue) {
        throw matchValue;
    }
}

export const result1 = divide1(100, 0);

Where throw matchValue is an EmitExpression.

2) Should we add a ThisExpr to Fable AST, so I can identify if this is being used as thisKeyword. I cannot be sure with Fable.IdentExpr is set to this if it's the this-keyword or not since those needs to be translated to self in Python. https://github.com/fable-compiler/Fable/blob/nagareyama/src/Fable.Transforms/Fable2Babel.fs#L792

alfonsogarciacaro commented 3 years ago

This is funny, throw was actually in Fable 2 AST but I removed it for Fable 3 in the effort to simplify the AST as it was only used in one place, but I guess we can add it again.

About "this", it's tricky. I need to check it again because in the F# AST this is represented differently in constructors or methods (there are some comments about this scattered through the code). There's an IsThisArgument property for identifiers in Fable AST, but let me check if this is only for the first argument of detached members or whether it also works for "actual this".

dbrattli commented 3 years ago

@alfonsogarciacaro I could use a little help with how to handle "internal" vs "user" compiled code e.g Exception have e.g .message, but how should I translate this? Can I be sure that user compiled code will never have a .message and will always instead end up as e.g Test__Message_229D3F39(x, e) so they will never interfere? Or do I have to wrap a catch'ed exception within e.g JsException object and add a .message property just to be sure? How do Fable do this from F# to Babel? Do you keep track of the types, or?

E.g: how does ex.Message.StartsWith in F# become ex.message.indexOf in JS? I will need to translate this to str(ex).index in Python (or wrap the objects).

alfonsogarciacaro commented 3 years ago

Actually we have an issue with F# exceptions (as in: exception Foo of int * int) I didn't fix before Fable 3 release šŸ˜… System.Exception is translated as JS Error so they do have a .message field. But F# exceptions are not deriving from JS Error for performance reasons (I think right now trying to access .Message just fails). There's a discussion about this somewhere. For your case and for general exception I think you can assume for now Fable accesses the .message and .stack properties.

dbrattli commented 3 years ago

How do we btw want the language selection to be in Fable? Today we can select e.g TypeScript using --typescript. So we could select Python using --python. But should we stop generating the .js then? We probably should as the generated JS might not be valid if e.g Emit is used. But sometimes we do want to see the JS. Should we have an option for --javascript, or should we require the user to run without --python to get JavaScript? @alfonsogarciacaro

ncave commented 3 years ago

@dbrattli --typescript only generates .ts files, so --python can generate just .py files, if you prefer. But technically they are all mutually exclusive, so should we instead introduce a new --language switch, with values JavaScript/TypeScript/Python?

dbrattli commented 3 years ago

@ncave Yes, I think it's a good idea something like [-lang|--language {"JavaScript"|"TypeScript"|"Python"}] where JavaScript could be default. I've already started adding a language property for the compiler, so I can try to fix the command line options as well? https://github.com/fable-compiler/Fable/pull/2345/files#diff-9cb94477ca17c7556e6f79d71ed20b71740376f7f3b00ee0ac3fdd7e519ac577R12

dbrattli commented 3 years ago

Some status. The Python support is getting better. Many language features are now in place. The huge remaining task is to port fable-library. To make this easier the fable-library for Python have been moved into the repo ,so we can convert and reuse the many fable-library .fs files to Python. However, many of them contains JS emitted code that needs to be intercepted and handled "manually". A test setup for Python (based on pytest) is in place currently with 43 tests passing, so this make it a lot easier to develop and add new features (and shows what is working).

Build fable-library for Python: dotnet fsi build.fsx library-py Run Python tests: dotnet fsi build.fsx test-py

ncave commented 3 years ago

@dbrattli Ideally we should try to convert more of fable-library to F#. List and Seq are already there, Array uses a small number of native methods, perhaps those can be separated.

On a slightly different topic, in your opinion what are the main benefits of implementing a new language codegen from Babel AST, instead of directly from Fable AST?

dbrattli commented 3 years ago

@ncave Python and JavaScript are quite close (imo) so it's relatively easy to rewrite Babel AST. I've ported a bit of JS to Python earlier e.g RxJS to RxPY. We also get the benefit of all the (bug-)fixes being made "upstream". That said, we could reuse most of Fable2Babel.fs and transform directly to Python using a Fable2Python.fs that combines/collapses Fable2Babel.fs and Babel2Python.fs. I actually thought about that earlier today. It then becomes more like a fork and bugfixes would most likely need to be applied to both places. For now I think it's ok to transform Babel AST, but eventually we might want to transform Fable AST directly.

alfonsogarciacaro commented 3 years ago

This is great @dbrattli! Thanks a lot for all your work. Actually, what I had in mind was putting most of Fable code in a library and release the different implementations as independent dotnet tools: fable(-js), fable-py (or any other name). Maybe that gives us more freedom in the release cycles, but I'm totally ok also with having a single dotnet tool that can compile to both language. That can be simpler, specially at the beginning.

About fable-library, yes, if possible instead of rewriting in python the current .ts files, it'd be nice to port them to F# instead . We should try to isolate the parts using Emit and adapt them to Python either by using #if:

#if FABLE_COMPILER_PYTHON
    [<Emit("print($0)")>]
    let log(x: obj) = ()

怀// Other native methods ...
#else
    [<Emit("console.log($0)")>]
    let log(x: obj) = ()

    // ...
#endif

Or we can add a new "multi-language" Emit attribute:

type EmitLangAttribute(macros: string[]) =
    inherit Attribute()

[<EmitLang([|"js:console.log($0)"; "py:print($0)"|])>]
let log(x: obj) = ()
dbrattli commented 3 years ago

Thanks @alfonsogarciacaro and @ncave , these are great ideas. I'll try them out to see what works. I see the benefit of translating fable-library to F# so I will try to help out here once the Python compiler is good enough to handle the files there. Using #if should help a lot, and we could probably avoid #ifs by moving the emits into language specific library files (since I have a separate .fsproj file for Python.

Zaid-Ajaj commented 3 years ago

@alfonsogarciacaro why not having two emit attributes instead? Emit for JS and EmitPy for python. The python transpiler should report a waening when it encounters emit for JS etc.

ncave commented 3 years ago

For my 2 cents, I like the current low cognitive load of having a single Emit attribute that simply emits. Imo what @dbrattli is doing right now makes sense (different project file for each fable-library language version). We can separate the different language emits in their own files, that implement well defined interfaces (or modules) to be called later from the common F# implementation.

ncave commented 3 years ago

A good example may be to convert the Array.Helpers into an interface (or just leave it as module) that can be implemented for each target language. Perhaps this effort of migrating more of fable-library to F# (as well as the --language compiler option) can go into a separate PR from this one, so it can go faster and be more easily contributed to.

alfonsogarciacaro commented 3 years ago

That's a good idea @ncave :+1: Actually we're already doing this when isolating native code in multiplatform Fable projects (.net and js). To simplify things I would probably just add a new Native.fs file to Fable.Library and have submodules there as needed, like:

namespace Fable.Library.Native            # or just Native

module Array = ..
module Map = ..

We should move there anything that uses Emit or values from Fable.Core.JS.

dbrattli commented 3 years ago

Working on async I'm trying to port Async.ts and AsyncBuilder.ts to F# i.e having Async.fs and AsyncBuilder.fs. But now I have the problem that my test file with code:

async { return () }
|> Async.StartImmediate

generates Python (JS code should be very similar) code:

startImmediate(singleton.Delay(lambda _=None: singleton.Return()))

AsyncBuilder however contains no methods but there's AsyncBuilder__Delay_458C1ECD. So I get AttributeError: 'AsyncBuilder' object has no attribute 'Delay'.

class AsyncBuilder:
    def __init__(self):
        namedtuple("object", [])()

def AsyncBuilder__ctor():
    return AsyncBuilder()

def AsyncBuilder__Delay_458C1ECD(x, generator):
    def lifted_17(ctx_1):
        def lifted_16(ctx):
            generator(None, ctx)

        protectedCont(lifted_16, ctx_1)

    return lifted_17
...

Any hints on how I can deal with this? @ncave @alfonsogarciacaro. Is there a way I can make F# classes generate classes with methods, or have my tests generate code that uses the static AsyncBuilder__Delay_458C1ECD instead of .Delay()?

ncave commented 3 years ago

@dbrattli Usually the answer to de-mangling is to use interfaces for the methods you want to not be mangled. On a related note, is async support one of those features that are perhaps better suited for a translation to native Python asyncio?

dbrattli commented 3 years ago

@ncave Ok, so that's how it works šŸ˜„ Thanks for the insight. I'll try to put the builder behind an interface and see if it helps (even if the F# builders are not interfaces it should probably work fine). As for async the plan was to use native Python asyncio (see here), but got problems with python cannot reuse already awaited coroutine, so I need to lazify them behind a function. Anyways I figured out I could just as well use a task or asyncio builder for Python native awaitables. The simplicify of AsyncBuilder.ts attracted me to try and port it to F#.

alfonsogarciacaro commented 3 years ago

Using an interface to avoid mangling should work as @ncave says. We also tried a couple of other approaches:

https://github.com/fable-compiler/Fable/blob/caa715f1156be29c8dd9b866a03031a1852b3186/src/fable-library/Map.fs#L524-L527

Then we can reference the methods using Naming.buildNameWithoutSanitation:

https://github.com/fable-compiler/Fable/blob/caa715f1156be29c8dd9b866a03031a1852b3186/src/Fable.Transforms/Replacements.fs#L1956-L1963

https://github.com/fable-compiler/Fable/blob/caa715f1156be29c8dd9b866a03031a1852b3186/src/Fable.Transforms/Replacements.fs#L1314-L1326

In that case you should only need to add the class to the replacedModules dictionary:

https://github.com/fable-compiler/Fable/blob/caa715f1156be29c8dd9b866a03031a1852b3186/src/Fable.Transforms/Replacements.fs#L3075


So unfortunately there's no consistent approach yet to link to fable-library code written in F# from Replacements. Maybe we use this opportunity to agree on one now.

dbrattli commented 3 years ago

Thanks for very interesting insights @ncave and @alfonsogarciacaro. With interfaces I stumbled upon another issue. How to deal with unit in Python? My interface looks like this:

type IAsyncBuilder =
    abstract member Bind<'T, 'U> : IAsync<'T> * ('T -> IAsync<'U>) -> IAsync<'U>
    abstract member Combine<'T> : IAsync<unit> * IAsync<'T> -> IAsync<'T>
    abstract member Delay<'T> : (unit -> IAsync<'T>) -> IAsync<'T>
    abstract member Return<'T> : value: 'T -> IAsync<'T>
   ...

The problem is that Return generates:

class AsyncBuilder:
    def Return(self, value):
        return protectedReturn(value)
    ....

This looks good and is probably good for JS, but in python you must give an argument if the function takes an argument. Thus you will get an error if you call x.Return() when 'T is unit. My first reaction was to make an overload:

abstract member Return : unit -> IAsync<unit>

but that has it's own problems. My current solution (which looks ugly is):

abstract member Return<'T> : [<ParamArray>] value: 'T [] -> IAsync<'T>

...

member this.Return<'T>([<ParamArray>] values: 'T []) : IAsync<'T> =
    if Array.isEmpty values then
        protectedReturn (unbox null)
    else
        protectedReturn values.[0]

But that requires me to have custom handling for every generic single argument function. Perhaps I should instead just make every single-argument function accept null (None in Python) as input. E.g:

class AsyncBuilder:
    def Return(self, value=None):
        return protectedReturn(value)
    ....

What would be the right way to deal with this problem?

alfonsogarciacaro commented 3 years ago

IIRC, for methods with unit -> X signature the calls have no arguments (as in the F# AST) but for lambdas or in this case generic arguments filled with unit calls/applications do have a unit argument in the Fable AST (also in the F# AST). This argument gets removed in the transformCallArgs helper of the Fable2Babel step. Maybe we could add a condition there and leave the unit argument when the target language is python:

https://github.com/fable-compiler/Fable/blob/caa715f1156be29c8dd9b866a03031a1852b3186/src/Fable.Transforms/Fable2Babel.fs#L1083-L1086

dbrattli commented 3 years ago

Hi @ncave, @alfonsogarciacaro , I'm could use some input on the handling of [<Inject>] parameters in Python. How is this done in Php @thinkbeforecoding? E.g functions like (in Array.fs):

let map (f: 'T -> 'U) (source: 'T[]) ([<Inject>] cons: Cons<'U>): 'U[] =
    let len = source.Length
    let target = allocateArrayFromCons cons len
    for i = 0 to (len - 1) do
        target.[i] <- f source.[i]
    target

My code is not generating the cons parameter and it's required for Python.

map(fn ar)

Is it optional for JS? How can I detect the attribute in the code and make it optinal? E.g

def map(f, source, cons=None):
    ...
thinkbeforecoding commented 3 years ago

Php also is explicit on optional parameters. I had to add a IsOptional flag on the argument model.

alfonsogarciacaro commented 3 years ago

There are two uses for Inject attribute in Fable. One is mainly experimental (to emulate somehow typeclasses and also to avoid having to inline a function to resolve generic type info) and you shouldn't care too much about it.

The other use is internal in fable-library methods that need some extra info passed by an argument resolved at compile time:

However, these functions are not called directly so Fable "cannot see" the Inject attribute. For this reason, we use the ReplacementsInject.fs file that says which functions in the library require injection. See how it's used: https://github.com/fable-compiler/Fable/blob/522f6aad211102271538798aeb90f4aed1f77dd6/src/Fable.Transforms/Replacements.fs#L988-L1019

This file was created automatically in the beginning with this script that detects which functions in Fable.Library have the last argument decorated with Inject. But I think at one point we stopped updating and IIRC the last updates to the ReplacementInjects. file have been done manually.

thinkbeforecoding commented 3 years ago

Knowing that, I can probably get rid of the IsOptional info... I'll check

dbrattli commented 3 years ago

@alfonsogarciacaro There seem to be an issue with injectArg for code such as:

type Id = Id of string

let inline replaceById< ^t when ^t : (member Id : Id)> (newItem : ^t) (ar: ^t[]) =
    Array.map (fun (x: ^t) -> if (^t : (member Id : Id) newItem) = (^t : (member Id : Id) x) then newItem else x) ar

let ar = [| {|Id=Id"foo"; Name="Sarah"|}; {|Id=Id"bar"; Name="James"|} |]
replaceById {|Id=Id"ja"; Name="Voll"|} ar |> Seq.head |> fun x -> equal "Sarah" x.Name
replaceById {|Id=Id"foo"; Name="Anna"|} ar |> Seq.head |> fun x -> equal "Anna" x.Name

Here the Array.map will not get the constructor injected. The code will transpile to JS without any injected arg:

return map((x) => (equals(newItem.Id, x.Id) ? newItem : x), ar);

Is this a bug? This code will work in JS (by luck?) but not for Python.

alfonsogarciacaro commented 3 years ago

Ah, sorry! Totally forgot about this but we have an "optimization" where the array constructor is not injected for the "standard" JS array: https://github.com/fable-compiler/Fable/blob/4ecab5549ab6fcaf317ab9484143420671ded43b/src/Fable.Transforms/Replacements.fs#L1005-L1009

We then just default to global Array when nothing is passed: https://github.com/fable-compiler/Fable/blob/4ecab5549ab6fcaf317ab9484143420671ded43b/src/fable-library/Array.fs#L25-L28

Not entirely sure why I did this, but I guess to save a few bytes in the bundled for the most common cases. We can remove this optimization for the next branch if it helps you.

dbrattli commented 3 years ago

Ok thanks. I'll try to see if I can get this working. I don't think we need to turn it off, we just need to generate an empty (null) value for the third argument so Python doesn't choke. i.e:

            | Types.arrayCons ->
                match genArg with
                | Number(numberKind,_) when com.Options.TypedArrays ->
                    args @ [getTypedArrayName com numberKind |> makeIdentExpr]
                | _ -> args @ [ Expr.Value(ValueKind.Null genArg, None) ]

For JS it will then generate:

map((x_3) => (equals(newItem_1.Id, x_3.Id) ? newItem_1 : x_3), ar, null))).Name);

and for Python:

def lifted_53(x_2):
    return newItem_1 if (equals(newItem_1["Id"], x_2["Id"])) else (x_2)

return map(lifted_53, ar, None)

Would something like that be an acceptable fix?

alfonsogarciacaro commented 3 years ago

That should work. Maybe we can use None instead. At one point we were removing None args in last position in Fable2Babel, it seems we are doing that now in FSharp2Fable, but we can add an extra check here: https://github.com/fable-compiler/Fable/blob/c54668b42b46c0538374b6bb2e283af41a6e5762/src/Fable.Transforms/Fable2Babel.fs#L1082-L1095

BTW, sorry I haven't checked this PR in depth šŸ˜… But if it's not breaking the tests, we could merge it soon to next as POC as we did with PHP. This is because I'm planning (time allowing) to do some refactoring to make it easier to target multiple languages with Fable, and It will probably easier if Python is already in next branch for that. We also avoid having to keep too many diverging branches in sync (main, next, python).

dbrattli commented 3 years ago

Ok @alfonsogarciacaro, I'll see if I can generate None instead of null. We could merge it soon. It's still breaking some of the existing Python tests but there's just a few ones left now and I should have that fixed within a week or so. Also need to create Fable.Core.PY.fs with the needed emits. The rewrite from Babel2Python -> Fable2Python was harder than I expected but I'm finally getting closer to be back on track again. I will cleanup the PR a bit and make it ready to be merged.

dbrattli commented 3 years ago

@alfonsogarciacaro I've fixed all the tests except one. It's however related to the same old issue I had with Babel šŸ˜± I need a way to know if Fable.Get is used on an AnonymousRecord type since they transform to Python dict, and I need to use subscript i.e [] access and not .dotted. The problem is that the left side can be anything, object, function call, ... so it's hard to know. Is there a better way. Should we have another GetKind for anonymous records, or what is the best way to do this?

I see this one in FSharp2Fable. How do I know later that the FieldGet was generated by an AnonRecord?

    // Getters and Setters
    | FSharpExprPatterns.AnonRecordGet(callee, calleeType, fieldIndex) ->
        let r = makeRangeFrom fsExpr
        let! callee = transformExpr com ctx callee
        let fieldName = calleeType.AnonRecordTypeDetails.SortedFieldNames.[fieldIndex]
        let typ = makeType ctx.GenericArgs fsExpr.Type
        return Fable.Get(callee, Fable.FieldGet(fieldName, false), typ, r)