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

add nested dict support #208

Merged
merged 20 commits into from
Feb 21, 2024
Merged
Show file tree
Hide file tree
Changes from 8 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
6 changes: 4 additions & 2 deletions lupa/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,10 @@ def _import_newest_lib():
raise RuntimeError("Failed to import Lupa binary module.")
# prefer Lua over LuaJIT and high versions over low versions.
module_name = max(modules, key=lambda m: (m[1] == 'lua', tuple(map(int, m[2] or '0'))))

_newest_lib = __import__(module_name[0], level=1, fromlist="*", globals=globals())
try:
_newest_lib = __import__(module_name[0], level=1, fromlist="*", globals=globals())
except ModuleNotFoundError:
_newest_lib = __import__(module_name[1], level=1, fromlist="*", globals=globals())
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems unrelated. Why did you change it?

Copy link
Contributor Author

@synodriver synodriver Jan 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually this is related to another bug. If you link to your custom lua libs, the original code will fail to find corret module. Reated to this issue

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then please revert it. You can propose it as a separate PR, if you think that it's the right thing to do. But it's unrelated to the change in this "nested dict" PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I will put it in a seperate pr.

return _newest_lib


Expand Down
74 changes: 53 additions & 21 deletions lupa/_lupa.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,11 @@ cdef object exc_info
from sys import exc_info

cdef object Mapping
cdef object Sequence
try:
from collections.abc import Mapping
from collections.abc import Mapping, Sequence
except ImportError:
from collections import Mapping # Py2
from collections import Mapping, Sequence # Py2

cdef object wraps
from functools import wraps
Expand Down Expand Up @@ -169,6 +170,10 @@ def lua_type(obj):
lua.lua_settop(L, old_top)
unlock_runtime(lua_object._runtime)

cdef inline int _len_as_int(Py_ssize_t obj) except -1:
if obj > <Py_ssize_t>INT_MAX:
raise OverflowError
return <int>obj

@cython.no_gc_clear
cdef class LuaRuntime:
Expand Down Expand Up @@ -520,15 +525,15 @@ cdef class LuaRuntime:
"""
return self.table_from(items, kwargs)

def table_from(self, *args):
def table_from(self, *args, bint recursive=False):
"""Create a new table from Python mapping or iterable.

table_from() accepts either a dict/mapping or an iterable with items.
Items from dicts are set as key-value pairs; items from iterables
are placed in the table in order.

Nested mappings / iterables are passed to Lua as userdata
(wrapped Python objects); they are not converted to Lua tables.
(wrapped Python objects) if `recursive` is False, they are not converted to Lua tables.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is what you mean, right?

Suggested change
(wrapped Python objects) if `recursive` is False, they are not converted to Lua tables.
(wrapped Python objects). If `recursive` is False, they are not converted to Lua tables.

"""
assert self._state is not NULL
cdef lua_State *L = self._state
Expand All @@ -538,12 +543,12 @@ cdef class LuaRuntime:
try:
check_lua_stack(L, 5)
lua.lua_newtable(L)
# FIXME: how to check for failure?
# FIXME: how to check for failure? and nested dict
for obj in args:
if isinstance(obj, dict):
for key, value in obj.iteritems():
py_to_lua(self, L, key, wrap_none=True)
py_to_lua(self, L, value)
py_to_lua(self, L, key, True, recursive)
py_to_lua(self, L, value, False, recursive)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Call arguments like None or False tend to be opaque – unless you know by heart which parameter they refer to, you have to look up the signature in order to understand what is requested. Better use keyword arguments. It's a C function call, so they are resolved at compile time.

Suggested change
py_to_lua(self, L, key, True, recursive)
py_to_lua(self, L, value, False, recursive)
py_to_lua(self, L, key, wrap_none=True, recursive=recursive)
py_to_lua(self, L, value, wrap_none=False, recursive=recursive)

lua.lua_rawset(L, -3)

elif isinstance(obj, _LuaTable):
Expand All @@ -559,12 +564,12 @@ cdef class LuaRuntime:
elif isinstance(obj, Mapping):
for key in obj:
value = obj[key]
py_to_lua(self, L, key, wrap_none=True)
py_to_lua(self, L, value)
py_to_lua(self, L, key, True, recursive)
py_to_lua(self, L, value, False, recursive)
lua.lua_rawset(L, -3)
else:
for arg in obj:
py_to_lua(self, L, arg)
py_to_lua(self, L, arg, False, recursive)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
py_to_lua(self, L, arg, False, recursive)
py_to_lua(self, L, arg, wrap_none=False, recursive=recursive)

lua.lua_rawseti(L, -2, i)
i += 1
return py_from_lua(self, L, -1)
Expand Down Expand Up @@ -1270,7 +1275,7 @@ cdef object resume_lua_thread(_LuaThread thread, tuple args):
# already terminated
raise StopIteration
if args:
nargs = len(args)
nargs = _len_as_int(len(args))
push_lua_arguments(thread._runtime, co, args)
with nogil:
status = lua.lua_resume(co, L, nargs, &nres)
Expand Down Expand Up @@ -1516,7 +1521,7 @@ cdef py_object* unpack_userdata(lua_State *L, int n) noexcept nogil:
cdef int py_function_result_to_lua(LuaRuntime runtime, lua_State *L, object o) except -1:
if runtime._unpack_returned_tuples and isinstance(o, tuple):
push_lua_arguments(runtime, L, <tuple>o)
return len(<tuple>o)
return _len_as_int(len(<tuple>o))
check_lua_stack(L, 1)
return py_to_lua(runtime, L, o)

Expand Down Expand Up @@ -1545,7 +1550,7 @@ cdef int py_to_lua_handle_overflow(LuaRuntime runtime, lua_State *L, object o) e
lua.lua_settop(L, old_top)
raise

cdef int py_to_lua(LuaRuntime runtime, lua_State *L, object o, bint wrap_none=False) except -1:
cdef int py_to_lua(LuaRuntime runtime, lua_State *L, object o, bint wrap_none=False, bint recursive=False, dict mapped_objs = None) except -1:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use consistent code style.

Suggested change
cdef int py_to_lua(LuaRuntime runtime, lua_State *L, object o, bint wrap_none=False, bint recursive=False, dict mapped_objs = None) except -1:
cdef int py_to_lua(LuaRuntime runtime, lua_State *L, object o, bint wrap_none=False, bint recursive=False, dict mapped_objs=None) except -1:

"""Converts Python object to Lua
Preconditions:
1 extra slot in the Lua stack
Expand Down Expand Up @@ -1597,13 +1602,40 @@ cdef int py_to_lua(LuaRuntime runtime, lua_State *L, object o, bint wrap_none=Fa
elif isinstance(o, float):
lua.lua_pushnumber(L, <lua.lua_Number><double>o)
pushed_values_count = 1
elif isinstance(o, _PyProtocolWrapper):
type_flags = (<_PyProtocolWrapper> o)._type_flags
o = (<_PyProtocolWrapper> o)._obj
pushed_values_count = py_to_lua_custom(runtime, L, o, type_flags)
elif recursive and isinstance(o, (list, Sequence)):
if mapped_objs is None:
mapped_objs = {}
if id(o) not in mapped_objs:
lua.lua_createtable(L, _len_as_int(len(o)), 0) # create a table at the top of stack, with narr already known
mapped_objs[id(o)] = lua.lua_gettop(L)
for i, v in enumerate(o, 1):
py_to_lua(runtime, L, v, wrap_none, recursive, mapped_objs)
lua.lua_rawseti(L, -2, i)
else: # self-reference detected
idx = mapped_objs[id(o)]
lua.lua_pushvalue(L, idx)
pushed_values_count = 1
elif recursive and isinstance(o, (dict, Mapping)):
if mapped_objs is None:
mapped_objs = {}
if id(o) not in mapped_objs:
lua.lua_createtable(L, 0, _len_as_int(len(o))) # create a table at the top of stack, with nrec already known
mapped_objs[id(o)] = lua.lua_gettop(L)
for key, value in o.items():
py_to_lua(runtime, L, key, wrap_none, recursive, mapped_objs)
py_to_lua(runtime, L, value, wrap_none, recursive, mapped_objs)
lua.lua_rawset(L, -3)
else: # self-reference detected
idx = mapped_objs[id(o)]
lua.lua_pushvalue(L, idx)
pushed_values_count = 1
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd reverse the conditions to put the simple "already mapped" case first.

Suggested change
elif recursive and isinstance(o, (list, Sequence)):
if mapped_objs is None:
mapped_objs = {}
if id(o) not in mapped_objs:
lua.lua_createtable(L, _len_as_int(len(o)), 0) # create a table at the top of stack, with narr already known
mapped_objs[id(o)] = lua.lua_gettop(L)
for i, v in enumerate(o, 1):
py_to_lua(runtime, L, v, wrap_none, recursive, mapped_objs)
lua.lua_rawseti(L, -2, i)
else: # self-reference detected
idx = mapped_objs[id(o)]
lua.lua_pushvalue(L, idx)
pushed_values_count = 1
elif recursive and isinstance(o, (dict, Mapping)):
if mapped_objs is None:
mapped_objs = {}
if id(o) not in mapped_objs:
lua.lua_createtable(L, 0, _len_as_int(len(o))) # create a table at the top of stack, with nrec already known
mapped_objs[id(o)] = lua.lua_gettop(L)
for key, value in o.items():
py_to_lua(runtime, L, key, wrap_none, recursive, mapped_objs)
py_to_lua(runtime, L, value, wrap_none, recursive, mapped_objs)
lua.lua_rawset(L, -3)
else: # self-reference detected
idx = mapped_objs[id(o)]
lua.lua_pushvalue(L, idx)
pushed_values_count = 1
elif recursive and isinstance(o, (list, Sequence)):
if mapped_objs is None:
mapped_objs = {}
if id(o) in mapped_objs:
obj = mapped_objs[id(o)]
lua.lua_pushvalue(L, obj)
else:
lua.lua_createtable(L, _len_as_int(len(o)), 0) # create a table at the top of stack, with narr already known
mapped_objs[id(o)] = lua.lua_gettop(L)
for i, v in enumerate(o, 1):
py_to_lua(runtime, L, v, wrap_none, recursive, mapped_objs)
lua.lua_rawseti(L, -2, i)
pushed_values_count = 1
elif recursive and isinstance(o, (dict, Mapping)):
if mapped_objs is None:
mapped_objs = {}
if id(o) in mapped_objs:
obj = mapped_objs[id(o)]
lua.lua_pushvalue(L, obj)
lua.lua_createtable(L, 0, _len_as_int(len(o))) # create a table at the top of stack, with nrec already known
mapped_objs[id(o)] = lua.lua_gettop(L)
for key, value in o.items():
py_to_lua(runtime, L, key, wrap_none, recursive, mapped_objs)
py_to_lua(runtime, L, value, wrap_none, recursive, mapped_objs)
lua.lua_rawset(L, -3)
pushed_values_count = 1

Actually, I'd even reuse the existing LuaRuntime.table_from() implementation here. Or, rather, extract a cdef function from it and use it in both places. This seems needlessly duplicated code.

else:
if isinstance(o, _PyProtocolWrapper):
type_flags = (<_PyProtocolWrapper>o)._type_flags
o = (<_PyProtocolWrapper>o)._obj
else:
# prefer __getitem__ over __getattr__ by default
type_flags = OBJ_AS_INDEX if hasattr(o, '__getitem__') else 0
# prefer __getitem__ over __getattr__ by default
type_flags = OBJ_AS_INDEX if hasattr(o, '__getitem__') else 0
pushed_values_count = py_to_lua_custom(runtime, L, o, type_flags)
return pushed_values_count

Expand Down Expand Up @@ -1809,7 +1841,7 @@ cdef object execute_lua_call(LuaRuntime runtime, lua_State *L, Py_ssize_t nargs)
lua.lua_replace(L, -2)
lua.lua_insert(L, 1)
has_lua_traceback_func = True
result_status = lua.lua_pcall(L, nargs, lua.LUA_MULTRET, has_lua_traceback_func)
result_status = lua.lua_pcall(L, <int>nargs, lua.LUA_MULTRET, has_lua_traceback_func)
if has_lua_traceback_func:
lua.lua_remove(L, 1)
results = unpack_lua_results(runtime, L)
Expand Down Expand Up @@ -1987,7 +2019,7 @@ cdef bint call_python(LuaRuntime runtime, lua_State *L, py_object* py_obj) excep
else:
args = ()
kwargs = {}

for i in range(nargs):
arg = py_from_lua(runtime, L, i+2)
if isinstance(arg, _PyArguments):
Expand Down
85 changes: 85 additions & 0 deletions lupa/tests/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -632,6 +632,91 @@ def test_table_from_table_iter_indirect(self):
self.assertEqual(list(table2.keys()), [1, 2, 3])
self.assertEqual(set(table2.values()), set([1, 2, "foo"]))

def test_table_from_nested_dict(self):
data = {"a": {"a": "foo"}, "b": {"b": "bar"}}
table = self.lua.table_from(data, recursive=True)
self.assertEqual(table["a"]["a"], "foo")
self.assertEqual(table["b"]["b"], "bar")
self.lua.globals()["data"] = table
self.lua.eval("assert(data.a.a=='foo', 'failed')")
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a test where we make sure that this raises an exception if the condition fails, so that the test actually fails?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I would add it later

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you've added such a test.
I guess you could also move the test assertions out of Lua and write something like this instead:

self.assertEqual(self.lua.eval("data.a.a"), 'foo')

Feel free to add a test helper method to make this shorter:

def assertLuaResult(lua_expression, result):
    self.assertEqual(self.lua.eval(lua_expression), result)

self.lua.eval("assert(data.b.b=='bar', 'failed')")
self.lua.eval("assert(type(data.a)=='table', 'failed, expect table, got '..type(data.a))")
self.lua.eval("assert(type(data.b)=='table', 'failed, expect table, got '..type(data.b))")
self.lua.execute("""function itertable(table)
for k,v in pairs(table) do
print(k)
if type(v) == "table" then
itertable(v)
else
print(v)
end
end
end
print('\\n')
itertable(data)
""")
del self.lua.globals()["data"]

def test_table_from_nested_list(self):
data = {"a": {"a": "foo"}, "b": [1, 2, 3]}
table = self.lua.table_from(data, recursive=True)
self.assertEqual(table["a"]["a"], "foo")
self.assertEqual(table["b"][1], 1)
self.assertEqual(table["b"][2], 2)
self.assertEqual(table["b"][3], 3)
self.lua.globals()["data"] = table
self.lua.eval("assert(data.a.a=='foo', 'failed')")
self.lua.eval("assert(#data.b==3, 'failed')")
self.lua.eval("assert(type(data.a)=='table', 'failed, expect table, got '..type(data.a))")
self.lua.eval("assert(type(data.b)=='table', 'failed, expect table, got '..type(data.b))")
self.lua.execute("""function itertable(table)
for k,v in pairs(table) do
print(k)
if type(v) == "table" then
itertable(v)
else
print(v)
end
end
end
print('\\n')
itertable(data)
""")
del self.lua.globals()["data"]

def test_table_from_nested_list_bad(self):
data = {"a": {"a": "foo"}, "b": [1, 2, 3]}
table = self.lua.table_from(data, recursive=True) # in this case, lua will get userdata instead of table
self.assertEqual(table["a"]["a"], "foo")
print(list(table["b"]))
self.assertEqual(table["b"][1], 1)
self.assertEqual(table["b"][2], 2)
self.assertEqual(table["b"][3], 3)
self.lua.globals()["data"] = table

self.lua.eval("assert(type(data.a)=='table', 'failed, expect table, got '..type(data.a))")
self.lua.eval("assert(type(data.b)=='table', 'failed, expect table, got '..type(data.b))")

del self.lua.globals()["data"]

def test_table_from_self_ref_obj(self):
data = {}
data["key"] = data
l = []
l.append(l)
data["list"] = l
table = self.lua.table_from(data, recursive=True)
self.lua.globals()["data"] = table
self.lua.eval("assert(type(data)=='table', '')")
self.lua.eval("assert(type(data['key'])=='table', '')")
self.lua.eval("assert(type(data['list'])=='table', '')")
self.lua.eval("assert(data['list']==data['list'][1], 'wrong self-ref list')")
self.lua.eval("assert(type(data['key']['key']['key']['key'])=='table', 'wrong self-ref map')")
self.lua.eval("assert(type(data['key']['key']['key']['key']['list'])=='table', 'wrong self-ref map')")
# self.assertEqual(table["key"], table)
# self.assertEqual(table["list"], table["list"][0])
del self.lua.globals()["data"]

# FIXME: it segfaults
# def test_table_from_generator_calling_lua_functions(self):
# func = self.lua.eval("function (obj) return obj end")
Expand Down