Skip to content

Improved Currency Module #127

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

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Improved Currency Module #127

wants to merge 7 commits into from

Conversation

AustinRow1
Copy link

I have implemented functionality that allows the user to use unofficial names for currencies. For example, rather than having to say 'USD to GBP' to get the conversion from US Dollars to British Pounds, the user can now say something like "usd to pounds" and get the same result.

The names that can be used to refer to different currency symbols can be found in data/currencies.json which has been added to store this data.

Note that until the AI has been trained more, there are some limitations to the usefulness of this. For example, the proper result will be returned if the user enters 'how much is us dollars to pounds', however, the AI will not be able to decipher the intent properly if given the request 'us dollars to pounds' or even more simply, 'us dollars to gbp'.

The issues with the AI are not unique to this new functionality, however, as some requests that should already work, like 'usd to cad rate', will not work as the AI will not provide a to_currency in the entities that it returns. Though a similar request of 'usd to inr rate' will work. This just demonstrates a general need for more training in recognizing currency requests for the AI.

…fficial abbreviations. For example, user can now send 'usd to pounds' and will receive conversion for 'usd to gbp'.
…fficial abbreviations. For example, user can now send 'usd to pounds' and will receive conversion for 'usd to gbp'.
…o currency-module-improvement

# Conflicts:
#	config.py
This is a file created by PyCharm when integrating version control into a project that you are working on. I apparently forgot to keep this from getting pushed so I'm deleting it now.
@swapagarwal
Copy link
Owner

@AustinRow1 This looks like a clean, and quite useful addition.
However, as you correctly pointed out, the AI needs to be trained to support this.

@@ -0,0 +1,34 @@
{
"USD": "USD", "DOLLAR": "USD", "BUCK": "USD", "US DOLLAR": "USD",
Copy link
Owner

Choose a reason for hiding this comment

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

Can there be a more concise way of storing this information?
I don't like USD being repeated.

Copy link
Author

Choose a reason for hiding this comment

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

I wrote it this way so that it would always require constant time to check for a translation. I could implement it in the following format:
{ "USD": ["DOLLAR", "BUCK", "US DOLLAR"] }

This would make it more concise, but it would also slow it down. The slowdown would be negligible in times with little usage, but if Jarvis ever came under heavy load it may be nice to have faster translation.

What do you think?

Copy link
Owner

Choose a reason for hiding this comment

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

A quick search gave this: http://stackoverflow.com/questions/2974022
Can you explore more on this? The ideal solution would have a constant time lookup for a key, while removing the duplication of values.

@swapagarwal
Copy link
Owner

Hey @AustinRow1, will you be willing to complete this? 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants