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] Import fixes #4035

Merged
merged 5 commits into from
Feb 9, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/Fable.Cli/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Added

* [Python] - Print root module and module function comments (by @alfonsogarciacaro)
* [Python] Print root module and module function comments (by @alfonsogarciacaro)
* [Rust] Add support for module comments (by @ncave)
* [Rust] Add support for null strings (by @ncave)

### Fixed

* [JS/TS] - Fix anonymous record printing (#4029) (by @alfonsogarciacaro)
* [Python] - Fix #3998: PhysicalEquality (by @alfonsogarciacaro)
* [Python] Resolve relative paths for non-qualified imports (#3481) (by @alfonsogarciacaro)
* [Python] `importSideEffects` shouldn't generate identifier (#3965) (by @alfonsogarciacaro)
* [JS/TS] Fix #4031: Hoist vars locally in for and while loops (@alfonsogarciacaro)

## 5.0.0-alpha.9 - 2025-01-28
Expand Down
4 changes: 1 addition & 3 deletions src/Fable.Cli/Pipeline.fs
Original file line number Diff line number Diff line change
Expand Up @@ -252,10 +252,8 @@ module Python =
|> Array.choose (fun part ->
i <- i + 1

if part = "." then
if part = "." || part = ".." then
None
elif part = ".." then
Some ""
elif i = parts.Length - 1 then
Some(normalizeFileName part)
else
Expand Down
2 changes: 2 additions & 0 deletions src/Fable.Compiler/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

* [JS/TS] - Fix anonymous record printing (#4029) (by @alfonsogarciacaro)
* [Python] - Fix #3998: PhysicalEquality (by @alfonsogarciacaro)
* [Python] Resolve relative paths for non-qualified imports (#3481) (by @alfonsogarciacaro)
* [Python] `importSideEffects` shouldn't generate identifier (#3965) (by @alfonsogarciacaro)
* [JS/TS] Fix #4031: Hoist vars locally in for and while loops (@alfonsogarciacaro)

## 5.0.0-alpha.9 - 2025-01-28
Expand Down
113 changes: 59 additions & 54 deletions src/Fable.Transforms/Python/Fable2Python.fs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ type ReturnStrategy =
type Import =
{
Module: string
LocalIdent: Identifier option
LocalIdent: Identifier
Name: string option
}

Expand Down Expand Up @@ -4249,17 +4249,17 @@ module Util =
match im.Name with
| Some "*"
| Some "default" ->
let (Identifier local) = im.LocalIdent.Value
let (Identifier local) = im.LocalIdent

if moduleName <> local then
Some moduleName, Alias.alias im.LocalIdent.Value
Some moduleName, Alias.alias im.LocalIdent
else
None, Alias.alias im.LocalIdent.Value
None, Alias.alias im.LocalIdent
| Some name ->
let name = Naming.toSnakeCase name

Some moduleName, Alias.alias (Identifier(Helpers.clean name), ?asname = im.LocalIdent)
| None -> None, Alias.alias (Identifier(moduleName), ?asname = im.LocalIdent)
Some moduleName, Alias.alias (Identifier(Helpers.clean name), asname = im.LocalIdent)
| None -> None, Alias.alias (Identifier(moduleName), asname = im.LocalIdent)
)
|> List.groupBy fst
|> List.map (fun (a, b) -> a, List.map snd b)
Expand Down Expand Up @@ -4290,16 +4290,9 @@ module Util =
let getIdentForImport (ctx: Context) (moduleName: string) (name: string option) =
// printfn "getIdentForImport: %A" (moduleName, name)
match name with
| None -> Path.GetFileNameWithoutExtension(moduleName) |> Identifier |> Some
| Some name ->
match name with
| "default"
| "*" -> Path.GetFileNameWithoutExtension(moduleName)
| _ -> name
|> Naming.toSnakeCase
|> getUniqueNameInRootScope ctx
|> Identifier
|> Some
| None -> Path.GetFileNameWithoutExtension(moduleName)
| Some name -> name |> Naming.toSnakeCase |> getUniqueNameInRootScope ctx
|> Identifier

module Compiler =
open Util
Expand All @@ -4317,47 +4310,59 @@ module Compiler =

member _.GetImportExpr(ctx, moduleName, ?name, ?r) =
// printfn "GetImportExpr: %A" (moduleName, name)
let cachedName = moduleName + "::" + defaultArg name "module"
let name =
match name with
| None
| Some null -> ""
| Some name -> name.Trim()

let isQualifiedPythonImport =
match name with
| ""
| "default"
| "*" -> false
| _ -> true

let cachedName =
moduleName
+ "::"
+ if isQualifiedPythonImport then
name
else
""

match imports.TryGetValue(cachedName) with
| true, i ->
match i.LocalIdent with
| Some localIdent -> Expression.identifier localIdent
| None -> Expression.none
| true, i -> i.LocalIdent |> Expression.identifier
| false, _ ->
let local_id = getIdentForImport ctx moduleName name

match name with
| Some "*"
| None ->
let i =
{
Name = None
Module = moduleName
LocalIdent = local_id
}

imports.Add(cachedName, i)
| Some name ->
let i =
{
Name =
if name = Naming.placeholder then
"`importMember` must be assigned to a variable" |> addError com [] r

name
else
name
|> Some
Module = moduleName
LocalIdent = local_id
}

imports.Add(cachedName, i)

match local_id with
| Some localId -> Expression.identifier localId
| None -> Expression.none
let local_id =
if isQualifiedPythonImport then
Some name
else
None
|> getIdentForImport ctx moduleName

let importName =
match name with
| _ when not isQualifiedPythonImport -> None
| Naming.placeholder ->
"`importMember` must be assigned to a variable" |> addError com [] r
Some name
| _ -> Some name

let i =
{
Name = importName
Module = moduleName
LocalIdent = local_id
}

imports.Add(cachedName, i)

// If the import member is empty we understand this is done for side-effects only
if name = "" then
Expression.none
else
Expression.identifier local_id

member _.GetAllImports() =
imports.Values :> Import seq |> List.ofSeq
Expand Down
6 changes: 6 additions & 0 deletions src/Fable.Transforms/Python/PythonPrinter.fs
Original file line number Diff line number Diff line change
Expand Up @@ -873,6 +873,12 @@ let run writer (program: Module) : Async<unit> =
| ImportFrom({ Module = Some(Identifier path) } as info) ->
let path = printer.MakeImportPath(path)
ImportFrom { info with Module = Some(Identifier path) }
| Import({ Names = names }) ->
let names =
names
|> List.map (fun n -> { n with Name = printer.MakeImportPath(n.Name.Name) |> Identifier })

Import { Names = names }
| decl -> decl
|> printDeclWithExtraLine false printer

Expand Down
40 changes: 40 additions & 0 deletions tests/Python/TestPyInterop.fs
Original file line number Diff line number Diff line change
Expand Up @@ -164,4 +164,44 @@ let factorial (count : int) : int =
let ``test emitPyStatement works with parameters`` () =
factorial 5 |> equal 120

[<Fact>]
let ``test importSideEffects`` () = // See #3965
importSideEffects "./native_code.py"
let mutable x = 3
Py.python $"""
{x} = native_code.add5({x} + 2)
"""
x |> equal 10

type NativeCode =
abstract add5: int -> int

[<Fact>]
let ``test importAll`` () =
let nativeCode: NativeCode = importAll "./native_code.py"
3 |> nativeCode.add5 |> equal 8

let add5 (x: int): int = importMember "./native_code.py"

[<Fact>]
let ``test importMember`` () =
add5 -1 |> equal 4

// Cannot use the same name as Fable will mangle the identifier
let add7: int -> int = importMember "./native_code.py"
add7 12 |> equal 19

let add5': int -> int = import "add5" "./native_code.py"
add5' 12 |> equal 17

let multiply3 (x: int): int = importMember "./more_native_code.py"
multiply3 9 |> equal 27

[<ImportAll("./native_code.py")>]
let nativeCode: NativeCode = nativeOnly

[<Fact>]
let ``test ImportAll works with relative paths`` () = // See #3481
3 |> nativeCode.add5 |> equal 8

#endif
2 changes: 2 additions & 0 deletions tests/Python/more_native_code.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
def multiply3(x: int) -> int:
return x * 3
6 changes: 6 additions & 0 deletions tests/Python/native_code.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
def add5(x: int) -> int:
return x + 5


def add7(x: int) -> int:
return x + 7
Loading