Skip to content

Preserve element order when initializing Python dictionary #57

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

Merged
merged 3 commits into from
Sep 1, 2022
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
38 changes: 34 additions & 4 deletions PythonKit/Python.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1265,8 +1265,8 @@ extension PythonObject : SignedNumeric {
return self < 0 ? -self : self
}

//override the default implementation of - prefix function
//from SignedNumeric (https://bugs.swift.org/browse/SR-13293)
// Override the default implementation of `-` prefix function
// from SignedNumeric (https://bugs.swift.org/browse/SR-13293).
public static prefix func - (_ operand: Self) -> Self {
return performUnaryOp(PyNumber_Negative, operand: operand)
}
Expand Down Expand Up @@ -1463,8 +1463,36 @@ extension PythonObject : ExpressibleByArrayLiteral, ExpressibleByDictionaryLiter
}
public typealias Key = PythonObject
public typealias Value = PythonObject

// Preserves element order in the final Python object, unlike
// `Dictionary.pythonObject`. When keys are duplicated, throw the same
// runtime error as `Swift.Dictionary.init(dictionaryLiteral:)`. This
// differs from Python's key uniquing semantics, which silently override an
// existing key with the next one it encounters.
public init(dictionaryLiteral elements: (PythonObject, PythonObject)...) {
self.init(Dictionary(elements, uniquingKeysWith: { lhs, _ in lhs }))
_ = Python // Ensure Python is initialized.
let dict = PyDict_New()!
for (key, value) in elements {
let k = key.ownedPyObject
let v = value.ownedPyObject

// Use Python's native key checking instead of querying whether
// `elements` contains the key. Although this could theoretically
// produce different results, it produces the Python object we want.
switch PyDict_Contains(dict, k) {
case 0:
PyDict_SetItem(dict, k, v)
case 1:
fatalError("Dictionary literal contains duplicate keys")
default:
try! throwPythonErrorIfPresent()
fatalError("No result or error checking whether \(elements) contains \(key)")
}

Py_DecRef(k)
Py_DecRef(v)
}
self.init(consuming: dict)
}
}

Expand Down Expand Up @@ -1867,7 +1895,9 @@ public struct PythonClass {
(key, value.pythonObject)
}

dictionary = Dictionary(castedElements, uniquingKeysWith: { lhs, _ in lhs })
dictionary = Dictionary(castedElements, uniquingKeysWith: { _, _ in
fatalError("Dictionary literal contains duplicate keys")
})
}
}

Expand Down
4 changes: 4 additions & 0 deletions PythonKit/PythonLibrary+Symbols.swift
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,10 @@ let PyErr_Fetch: @convention(c) (
let PyDict_New: @convention(c) () -> PyObjectPointer? =
PythonLibrary.loadSymbol(name: "PyDict_New")

let PyDict_Contains: @convention(c) (
PyObjectPointer?, PyObjectPointer?) -> Int32 =
PythonLibrary.loadSymbol(name: "PyDict_Contains")

let PyDict_SetItem: @convention(c) (
PyObjectPointer?, PyObjectPointer, PyObjectPointer) -> Void =
PythonLibrary.loadSymbol(name: "PyDict_SetItem")
Expand Down
41 changes: 34 additions & 7 deletions Tests/PythonKitTests/PythonRuntimeTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class PythonRuntimeTests: XCTestCase {
XCTAssertEqual(2, Python.len(dict))
XCTAssertEqual(1, dict["a"])
XCTAssertEqual(0.5, dict[1])

XCTAssertEqual(2, dict.count as Int)
XCTAssertEqual(2, dict.checking.count!)
XCTAssertEqual(2, dict.throwing.count!)
Expand All @@ -43,6 +43,33 @@ class PythonRuntimeTests: XCTestCase {
XCTAssertEqual("c", dict["b"])
dict["b"] = "d"
XCTAssertEqual("d", dict["b"])

// Dictionary initializer patch does not work on Python 2, but that
// version is no longer being actively supported.
guard Python.versionInfo.major >= 3 else {
return
}

// Pandas DataFrame regression test spotted in Jupyter. This is
// non-deterministic, so repeat it several times to ensure the bug does
// not appear.
for _ in 0..<15 {
let records: [PythonObject] = [
["col 1": 3, "col 2": 5],
["col 1": 8, "col 2": 2]
]
let records_description =
"[{'col 1': 3, 'col 2': 5}, {'col 1': 8, 'col 2': 2}]"
XCTAssertEqual(String(describing: records), records_description)

let records_alt: [PythonObject] = [
["col 1": 3, "col 2": 5, "col 3": 4],
["col 1": 8, "col 2": 2, "col 3": 4]
]
let records_alt_description =
"[{'col 1': 3, 'col 2': 5, 'col 3': 4}, {'col 1': 8, 'col 2': 2, 'col 3': 4}]"
XCTAssertEqual(String(describing: records_alt), records_alt_description)
}
}

func testRange() {
Expand Down Expand Up @@ -123,12 +150,12 @@ class PythonRuntimeTests: XCTestCase {
}

func testUnaryOps() {
var x = PythonObject(5)
x = -x
XCTAssertEqual(-5, x)
x = PythonObject(-5)
x = -x
XCTAssertEqual(5, x)
var x = PythonObject(5)
x = -x
XCTAssertEqual(-5, x)
x = PythonObject(-5)
x = -x
XCTAssertEqual(5, x)
}

func testComparable() {
Expand Down