traverseda / mycroft-skill-unitconversion

Use python-pint to do unit conversions
2 stars 5 forks source link

Error when trying to convert fluid ounces into one cup #3

Open mexican1973 opened 4 years ago

mexican1973 commented 4 years ago

Hi I am getting an error when trying to convert fluid ounces in one cup. "How many fluid onces are in one cup?" Both are form the same dimension so it should be possible. I think the error handling should be improved to explain what the exact error is. This is what I get out of the skills.log: 2020-02-15 12:45:29.180 | ERROR | 769 | mycroft.skills.mycroft_skill.mycroft_skill:on_error:805 | An error occurred while processing a request in Unit Conversion Skill Traceback (most recent call last): File "/opt/mycroft/skills/mycroft-skill-unitconversion.traverseda/init.py", line 20, in handle_unit_conversion result = round((count*ureg(source)).to(target),2) File "/home/pi/mycroft-core/.venv/lib/python3.7/site-packages/pint/registry.py", line 1122, in parse_expression lambda x: self._eval_token( File "/home/pi/mycroft-core/.venv/lib/python3.7/site-packages/pint/pint_eval.py", line 92, in evaluate left = self.left.evaluate(define_op, bin_op, un_op) File "/home/pi/mycroft-core/.venv/lib/python3.7/site-packages/pint/pint_eval.py", line 102, in evaluate return define_op(self.left) File "/home/pi/mycroft-core/.venv/lib/python3.7/site-packages/pint/registry.py", line 1123, in x, case_sensitive=case_sensitive, use_decimal=use_decimal, **values File "/home/pi/mycroft-core/.venv/lib/python3.7/site-packages/pint/registry.py", line 1081, in _eval_token {self.get_name(token_text, case_sensitive=case_sensitive): 1} File "/home/pi/mycroft-core/.venv/lib/python3.7/site-packages/pint/registry.py", line 612, in get_name raise UndefinedUnitError(name_or_alias) pint.errors.UndefinedUnitError: 'one' is not defined in the unit registry

During handling of the above exception, another exception occurred:

Traceback (most recent call last): File "/home/pi/mycroft-core/mycroft/skills/mycroft_skill/event_container.py", line 66, in wrapper handler(message) File "/opt/mycroft/skills/mycroft-skill-unitconversion.traverseda/init.py", line 31, in handle_unit_conversion self.speak_dialog("unknownUnitError",{"units":e.unit_names}) AttributeError: 'UndefinedUnitError' object has no attribute 'unit_names' 2020-02-15 12:46:57.058 | ERROR | 769 | mycroft.skills.mycroft_skill.mycroft_skill:on_error:805 | An error occurred while processing a request in Unit Conversion Skill Traceback (most recent call last): File "/opt/mycroft/skills/mycroft-skill-unitconversion.traverseda/init.py", line 20, in handle_unit_conversion result = round((count*ureg(source)).to(target),2) File "/home/pi/mycroft-core/.venv/lib/python3.7/site-packages/pint/quantity.py", line 561, in to other = to_units_container(other, self._REGISTRY) File "/home/pi/mycroft-core/.venv/lib/python3.7/site-packages/pint/util.py", line 832, in to_units_container return registry._parse_units(unit_like) File "/home/pi/mycroft-core/.venv/lib/python3.7/site-packages/pint/registry.py", line 1167, in _parse_units return super()._parse_units(input_string, as_delta) File "/home/pi/mycroft-core/.venv/lib/python3.7/site-packages/pint/registry.py", line 1052, in _parse_units cname = self.get_name(name) File "/home/pi/mycroft-core/.venv/lib/python3.7/site-packages/pint/registry.py", line 612, in get_name raise UndefinedUnitError(name_or_alias)

ChanceNCounter commented 4 years ago

From the programmer end, the error handling says exactly what went wrong with the conversion.

From the user's perspective, it looks like another error occurred in the error handling. Otherwise the skill has Mycroft tell you what happened.

@traverseda : Shot in the dark, I think the reported utterance is being captured by the wrong vocab line, shoving words into the wrong variables.

traverseda commented 4 years ago

I've had a fair number of issues with mycroft's intent parsers capturing things in weird ways, over capturing, capturing values that I never defined, and occasionally capturing things in weird ways.

Unfortunately I don't have enough time to really dig into this with my current work load, and I'm not actually a mycroft user this was just an experiment.

If anyone is interested in more actively maintaining this please do reach out, and of course pull requests are welcome. I would have liked to extend this into a more useful command-line math/units system, but I found the intent parser(s) too inconsistent to work with for a hobby project.

traverseda commented 4 years ago

Also @mexican1973 somehow this repo dropped off my watch list, sorry about that.

ChanceNCounter commented 4 years ago

I don't know if I'll have time to work on it soon, so if anyone else reading this is interested, go for it. However, I have starred the repo with the intention (hah!) of at some point fixing the intent.

ChanceNCounter commented 4 years ago

Oh, and another also @mexican1973 for the time being, if you remove this skill, Mycroft should pass such requests through to Wolfram Alpha, giving you the correct conversion. It'll just be substantially slower than this skill, is all.

ChanceNCounter commented 4 years ago

I should probably also have mentioned that the intent parsers have made strides. It's frequently regex expansion these days, when something is overcapturing. I'm beginning to lean toward fewer input variables in favor of parsing longer input values, using the language parsers.

For example, if I do touch this intent, I'll probably replace the {number} {unit} bit with {value1} and then use lingua_franca.extract_number() on value1, leaving me with [someNumber, "someUnit"] that I'll pass to Pint.

traverseda commented 4 years ago

Yeah, when I first wrote this I wasn't aware of lingua_franca, I'm not entirely sure it was documented yet. It seems like a great solution.

I actually did write my own custom intent parser with "verbal expressions", a more verbose regex format. That worked quite a bit better than what I was doing with mycroft's intents, but was very dependent on definition order which would have made it very difficult to translate the skill into toher languages.

Wish I had time to pick this stuff back up, but my docket is pretty full with paying contract work. This is definitely only a proof of concept right now.

ChanceNCounter commented 4 years ago

In your unnecessary defense, I'm not sure LF existed as a separate repo when you wrote this. It derives from all the language functions that used to be in Mycroft-core, but those probably weren't fleshed out, either.

If I get around to this, I'll hit you up with a PR.