Add evaluate_mathematically #80

DarkmatterVale · 2015-11-23T00:46:53Z

Here is my first attempt at adding a way to evaluate mathematical expressions. @gunthercox what do you think? What are your thoughts on improving implementation?

A couple of notes. This is not finished, but I am at a point where I need to get input on how to implement it into the program. It currently looks at every incoming question or statement and attempts to decide whether or not the point is to evaluate a mathematical expression. This sort of works, but you run into issues pretty easily once you start describing the quantity of an object. It might think you are trying to have a mathematical expression evaluated, and then it will give the incorrect answer.

Another thing to note is the way I structured the function. I decided to create a new kind of adapter because this is definitely not an io adapter, nor is it a storage adapter. You could argue it is a logic adapter, but it doesn't work independent of another logic adapter.

Also, the reason the build is failing is because the answer in Python 3.x comes out to 1.0 while in 2.7 it is 1.

Doesn’t actually do anything yet, but it works :)

The class can now evaluate basic expressions & successfully completes tests

More dynamic input is allowed

Now, it can successfully take any size number that is represented in word ( if they are in the table, which is the next thing to improve )

- Updated storage to be more dynamic - Removed error causing print statement - Finished tests

gunthercox · 2015-11-23T13:50:48Z

@DarkmatterVale This is awesome. I will go over this pull request and provide some thorough feedback as soon as possible.

So far I think I like the concept of the processor adapter. This might be a really good addition in the future for determining if logic adapters need to run in other multi-logic adapter scenarios.

DarkmatterVale · 2015-11-24T00:41:23Z

I had a similar thought process and agree that this could be very useful in the future. This will come especially in handy when you need a logic adapter in addition to a context adapter (or other kinds of adapters).

DarkmatterVale · 2015-11-24T16:26:42Z

I have renamed the addition so that evaluate_mathematically() is now a plugin. This is a much better name for the function's purpose.

Also, plugins should allow for "external" functionality to be implemented, while Logic adapters should act as the way the user interfaces with the chatbot. In the future, we can use the plugins directory to add new functions such as sending emails, reading twitter messages, etc.

- Updated implementation, making it more dynamic - Expansion for future plugins is easy now - Cleaned up evaluate_mathematically() tests

gunthercox · 2015-11-25T00:02:01Z

chatterbot/adapters/plugins/evaluate_mathematically.py

+        it returns False.
+        """
+
+        if string.isdigit():


Will this handle negative integers?

If not, it should be possible to detect them like this:

string.lstrip('-').isdigit()

Yes, I have made sure that it will handle negative integers without any issues

We are lacking tests for negative numbers, which is why I'm also curious about how they are being handled.

isdigit will return False if there are any non-digits (including decimals and the negative sign) in the string.

I added tests in my next commit. They were previously being treated as floats, which I have now changed by replacing the is_integer() code with:

try: return int( string ) except: return False

DarkmatterVale · 2015-11-26T01:29:36Z

tests/test_chatbot_output.py

@@ -207,4 +224,3 @@ def test_database_is_not_updated_when_read_only(self):

        self.assertFalse(exists_before)
        self.assertFalse(exists_after)
-


I'm not sure why this is happening with the last line of a file...I will look at how to fix this with Atom within the next week and incorporate additional lines into a future commit.

This looks like Atom was removing the extra blank line at the end of the file.

There were two blank lines before, now there is only one.

That makes more sense than my theory :)

DarkmatterVale · 2015-11-29T15:33:53Z

@gunthercox and @kevin-brown Do you guys have any more questions? Do I need to give a more in-depth explanation for how the code works?

gunthercox · 2015-11-29T16:52:38Z

I'm checking this out locally using examples/terminal_example.py. It appears that when I enter a value that contains a mathematical expression, such as "What is 5 + 5?", no response is returned. This is strange since I can clearly see that tests are passing.

DarkmatterVale · 2015-11-29T17:52:58Z

I had the same problem. In fact, I am unable to see any changes I make to the code when I run the examples ( just in general ). The best way to test, which is the way I do it because of this issue, is to add a print statement right before the return statement. Then rerun the tests with --nocapture. The before and after code is below.

Before:

if not plugin_response is False:
     return plugin_response

After:

if not plugin_response is False:
     print plugin_response
     return plugin_response

The problem ( since I ran into the same issue on other projects ) is that the examples use the version of chatterbot currently installed on the computer, and not the local repository. Have you ever encountered the same situation?

gunthercox · 2015-11-29T18:52:08Z

In general, a good way to test out changes to python packages that are also imported in other local projects is just to install the package (with the changes you have made) locally.

This can be done using:

pip install . --upgrade

This should be run from within the same directory as setup.py.

Also, another item I noticed is that you will need to include the json data file you created in the installed package. This can be done by adding the following line to MANIFEST.in:

recursive-include chatterbot/adapters/* *.json

DarkmatterVale · 2015-11-29T20:10:19Z

Thank you for that information! I will not make these mistakes again; sorry for not doing things properly in the first place!

I was able to test ( correctly this time ) the adapter and I found the problem. I was not processing the response properly, leading it to not being displayed. This is fixed in the next commit along with the MANIFEST.in update. Please let me know if this fixes the problem on your machine!

gunthercox · 2015-11-29T21:29:37Z

No problem, this is all really good work and its cool to see it coming together. Also, that corrected the issue.

DarkmatterVale · 2015-11-29T21:34:01Z

Awesome, glad that fixed the issue! I will remember all of this next time I make changes so you guys don't have to spend hours reviewing code :D

How does it work? I know it doesn't work for all math expressions ( far from it ), but it is a start and a place to work from.

gunthercox · 2015-11-29T21:53:43Z

Code review is a part of programming. It's always possible to make a mistake, having a few more sets of eyes on it never hurts.

So far everything that I'm seeing here looks great. Performance-wise it works excellently. At the moment I believe there is only one more thing in the code that I believe could be improved, I'll leave a comment about it on the line.

DarkmatterVale · 2015-11-30T22:31:04Z

chatterbot/adapters/plugins/evaluate_mathematically.py

+
+        string = ''
+
+        for chunk in input_text.split( ' ' ):


Should I use input_text.split() instead of input_text.split( ' ' )? That way I would be separating the words based on all whitespace characters not just spaces.

Yup, input_text.split() is the way to go here 👍

gunthercox · 2015-12-02T21:32:03Z

@DarkmatterVale These changes look good to me. Did you have any more additions before I merge this?

DarkmatterVale · 2015-12-03T13:46:27Z

No I do not have any additional changes to make.

gunthercox · 2015-12-04T01:17:44Z

Alright, just did a manual merge with master because there was some conflicts that GitHub couldn't resolve automatically. Either way, this is an awesome addition and it will be available in the next release on pypi.

DarkmatterVale added 6 commits November 12, 2015 07:43

Created preprocessor & evaluate_mathematically()

08ede07

First working implementation of evaluate_mathematically

0bd770b

Doesn’t actually do anything yet, but it works :)

Successful evaluation of basic expressions

05450e0

The class can now evaluate basic expressions & successfully completes tests

Continued improvement to the math parser

f0c511c

More dynamic input is allowed

Continued improvement on parser

71a03d9

Now, it can successfully take any size number that is represented in word ( if they are in the table, which is the next thing to improve )

Finalized first working version of the parser

9c4f552

- Updated storage to be more dynamic - Removed error causing print statement - Finished tests

Expanded english math words

e469f4f

DarkmatterVale added 2 commits November 24, 2015 11:14

Rename to plugins

e1d9219

Rename to plugins

d77669a

DarkmatterVale added 2 commits November 24, 2015 13:53

Plugin update

fee0369

- Updated implementation, making it more dynamic - Expansion for future plugins is easy now - Cleaned up evaluate_mathematically() tests

Fix another import error & added comments

88e273f

gunthercox reviewed Nov 25, 2015
View reviewed changes

DarkmatterVale added 2 commits November 25, 2015 19:57

Fix miscellaneous errors & clean code up

e59570e

Continued code clean up

8e49176

DarkmatterVale reviewed Nov 26, 2015
View reviewed changes

is_integer update & added additional tests

366ebaf

Implement proper response processing

f3ce77b

DarkmatterVale reviewed Nov 30, 2015
View reviewed changes

Additional code clean up

1fef0ca

gunthercox closed this Dec 4, 2015

gunthercox deleted the evaluate_mathematically branch December 4, 2015 01:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add evaluate_mathematically #80

Add evaluate_mathematically #80

DarkmatterVale commented Nov 23, 2015

gunthercox commented Nov 23, 2015

DarkmatterVale commented Nov 24, 2015

DarkmatterVale commented Nov 24, 2015

gunthercox Nov 25, 2015

DarkmatterVale Nov 26, 2015

kevin-brown Nov 26, 2015

DarkmatterVale Nov 26, 2015

DarkmatterVale Nov 26, 2015

kevin-brown Nov 26, 2015

DarkmatterVale Nov 26, 2015

DarkmatterVale commented Nov 29, 2015

gunthercox commented Nov 29, 2015

DarkmatterVale commented Nov 29, 2015

gunthercox commented Nov 29, 2015

DarkmatterVale commented Nov 29, 2015

gunthercox commented Nov 29, 2015

DarkmatterVale commented Nov 29, 2015

gunthercox commented Nov 29, 2015

DarkmatterVale Nov 30, 2015

gunthercox Dec 1, 2015

gunthercox commented Dec 2, 2015

DarkmatterVale commented Dec 3, 2015

gunthercox commented Dec 4, 2015

		@@ -207,4 +224,3 @@ def test_database_is_not_updated_when_read_only(self):

		self.assertFalse(exists_before)
		self.assertFalse(exists_after)

Add evaluate_mathematically #80

Add evaluate_mathematically #80

Conversation

DarkmatterVale commented Nov 23, 2015

gunthercox commented Nov 23, 2015

DarkmatterVale commented Nov 24, 2015

DarkmatterVale commented Nov 24, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DarkmatterVale commented Nov 29, 2015

gunthercox commented Nov 29, 2015

DarkmatterVale commented Nov 29, 2015

gunthercox commented Nov 29, 2015

DarkmatterVale commented Nov 29, 2015

gunthercox commented Nov 29, 2015

DarkmatterVale commented Nov 29, 2015

gunthercox commented Nov 29, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gunthercox commented Dec 2, 2015

DarkmatterVale commented Dec 3, 2015

gunthercox commented Dec 4, 2015