-
-
Notifications
You must be signed in to change notification settings - Fork 121
Closed
Labels
bugSomething isn't workingSomething isn't working
Description
Summary
When using the API, repeated calls to Table.convert()
do not work correctly since all conversions quietly use the callable (function, lambda) from the first call to convert()
only. Subsequent invocations with different callables use the callable from the first invocation only.
Example
from sqlite_utils import Database
db = Database(memory=True)
table = db['table']
col = 'x'
table.insert_all([{col: 1}])
print(table.get(1))
table.convert(col, lambda x: x*2)
print(table.get(1))
def zeroize(x):
return 0
#zeroize = lambda x: 0
#zeroize.__name__ = 'zeroize'
table.convert(col, zeroize)
print(table.get(1))
Output:
{'x': 1}
{'x': 2}
{'x': 4}
Expected:
{'x': 1}
{'x': 2}
{'x': 0}
Explanation
This is some relevant documentation.
Table.convert()
takes aCallable
to perform data conversion on a column- The
Callable
is passed toDatabase.register_function()
Database.register_function()
uses the callable's__name__
attribute for registration- (Aside: all lambdas have a
__name__
of<lambda>
: I thought this was the problem, and it was close, but not quite) - However
convert()
first wraps the callable by local functionconvert_value()
- Consequently
register_function()
sees nameconvert_value
for all invocations fromconvert()
register_function()
silently ignores registrations using the same name, retaining only the first such registration
There's a mismatch between the comments and the code:
sqlite-utils/sqlite_utils/db.py
Line 404 in fc221f9
:param replace: set ``True`` to replace an existing function with the same name - otherwise throw an error |
but actually the existing function is returned/used instead (as the "registering custom sql functions" doc I linked above says too). Seems like this can be rectified to match the comment?
Suggested fix
I think there are four things:
- The call to
register_function()
fromconvert()
should have an explicitname=
parameter (to continue usingconvert_value()
and the progress bar). - For functions, this name can be the real function name. (I understand the sqlite api needs a name, and it's nice if those are recognizable names where possible). For lambdas would
'lambda-{uuid}'
or similar be acceptable? register_function()
really should throw an error on repeated attempts to register a duplicate (function, arity)-pair.- A test? I haven't looked at the test framework here but seems this should be testable.
See also
Metadata
Metadata
Assignees
Labels
bugSomething isn't workingSomething isn't working