search

wolph / alfred-converter

Alfred unit converter is a smart calculator for Alfred with support for unit conversions to make it a bit comparable to the Google Calculator and Wolfram Alpha.
93 stars 15 forks source link

Bug: safemath functions not working [with fix] #28

Closed lchmtw closed 4 years ago

lchmtw commented 4 years ago

While testing with math functions like 'log' and 'ceil', I have found that the workflow does not work as expected (gives 0 for everything).

Not sure if I am the only one experiencing this, but I was able to fix it by replacing this line (line 12 in converter/safe_math.py): safe_dict[k] = lambda *a: decimal.Decimal(getattr(math, k)(*a)) by this line safe_dict[k] = getLambda(k)

where

def getLambda(k):
    return lambda *a: decimal.Decimal(getattr(math, k)(*a))

According to my understanding, this has something to do with variable referencing in python. All lambda functions created under that loop referred to the same variable k, so as the value of k change every iteration, all previous created lambda functions is affected. So essentially that loop results in a bunch of copies of the last lambda function. By wrapping it by another function, the variable is forced to be copied.

Hope this helps someone.

wolph commented 4 years ago

Good catch, that's indeed an annoying bug. For some reason even with your fix I get weird issues. When I run log(10) it calls lgamma(10) internally and I'm not sure why...

I'm working on fixing the bug in any case, thanks for the help!

wolph commented 4 years ago

I found why the tests didn't trip. Several of the methods are natively supported by the decimal class or have custom implementations so they work around the regular implementations. Basically... it only failed when running a math function that had no decimal specific function.

In any case, I've fixed the issues in the new release :)