search

tchar / ulauncher-albert-calculate-anything

A ULauncher/Albert extension that supports currency, units and date time conversion, as well as a calculator that supports complex numbers and functions.
MIT License
103 stars 13 forks source link

Fix currency and unit conversion #46

Closed nathan818fr closed 3 months ago

nathan818fr commented 1 year ago

Fix #42 and fix #45 by removing the ZWD currency alias. Also filter currency names (>= 3 chars, only letters and not colliding with existing units) to prevent bugs with new crypto currencies.

Note: The only exception is ETH which is allowed to override exathou because it's a very common crypto currency (and exathou is an almost unknown unit).

igorsantos07 commented 10 months ago

Glad ULauncher is based on Github - would you merge your own branch into your master, so we can start using your fork instead of the dead project?

AlfredPianist commented 8 months ago

It's a shame that the project maintainer looks like abandoned the project. This PR can't be merged because @nathan818fr is not an official contributor to the project. I had to manually copy and paste the changes in the code to make it work. This plugin is awesome and thanks @nathan818fr for the fix!!

By the way. How did you know it was exactly that currency alias that crashed everything? If I may ask.

igorsantos07 commented 8 months ago

That's why I suggested Nathan to merge into his master, so we can use his fork instead of this zombie :)

But I guess he's also not looking around :(

tchar commented 5 months ago

I had some time to look into this.

I also found that ZWD was causing the issues but the underlying bug was in PintDefinitionParser._define_currency and more specifically at definition = definition or 'nan currency EUR'

It should be definition = definition or 'nan currency_EUR' The problem with the space is that Pint understands it as nan currency * EUR and tries to find the currency unit when activating the currency context.

It took me a long time to figure it out since I have not dealt with the project lately, but how it works is as follows: When you do

with unit_registry.context('currency'):
    ...

What Pint does behind the hood is to find all the definitions from the unit registry. Because ZWD was registered IN the registry as nan currency EUR = nan current * EUR even though this was updated in the context later as a proper value (e.g currency_ZWD = 0.002.... currency_EUR) it still did try to find all units from the unit registry and because in registry ZWD never got a value equivalence to EUR (only in context it did) it crashed saying "I cannot find a currency unit.

You could open a ticket on Pint's webpage because this behavior should have been caught when doing unit_registry.define(....) and not when activating a context.

PR #48 fixes this.

tchar commented 5 months ago

@nathan818fr As far as the currency filtering, I would like to merge your first commit that filters currencies. Would you mind merging the current master and making a new PR with the filtering? Just note there is no need to filter out ZWD anymore unless overridden by a higher-priority currency.

Also, merge to development in your PR.

tchar commented 3 months ago

Fixed by #48 please feel free to post the currency filtering mechanism in a new PR.