Add new parsing apis #724
Add new parsing apis #724
Conversation
adrienball
force-pushed
the
task/new-parsing-apis
branch
from
76cfd38 to
3e27bd5
Compare
adrienball
force-pushed
the
task/new-parsing-apis
branch
from
1b033fa to
ec1a307
Compare
Codecov Report
@@ Coverage Diff @@
## develop #724 +/- ##
===========================================
+ Coverage 88.16% 88.44% +0.27%
===========================================
Files 65 66 +1
Lines 3853 3963 +110
Branches 735 765 +30
===========================================
+ Hits 3397 3505 +108
- Misses 347 349 +2
Partials 109 109 |
adrienball
force-pushed
the
task/new-parsing-apis
branch
from
ec1a307 to
cc4bef5
Compare
| @@ -73,24 +69,29 @@ def classproperty(func): | |||
| # pylint: enable=invalid-name | |||
|
|
|||
|
|
|||
| def type_error(expected_type, found_type): | |||
| return TypeError("Expected %s but found: %s" % (expected_type, found_type)) | |||
| def type_error(expected_type, found_type, object_label=None): | |||
There was a problem hiding this comment.
I find the naming a bit misleading now.
|
|
||
|
|
||
| def missing_key_error(key, object_label=None): | ||
| if object_label is None: | ||
| return KeyError("Missing key: '%s'" % key) | ||
| return KeyError("Expected %s to have key: '%s'" % (object_label, key)) | ||
| raise DatasetFormatError("Missing key: '%s'" % key) |
There was a problem hiding this comment.
I find the naming of the function a bit misleading now.
snips_nlu/exceptions.py
Outdated
| format""" | ||
|
|
||
|
|
||
| class IntentFormatError(SnipsNLUError): |
There was a problem hiding this comment.
Shouldn't these last 2 inherit from the DatasetFormatError ?
| @@ -116,33 +116,49 @@ def get_intent(self, text, intents_filter=None): | |||
| NotTrained: When the intent classifier is not fitted | |||
|
|
|||
| """ | |||
| intents_results = self._get_intents(text, intents_filter) | |||
| if not intents_results or intents_results[0][RES_INTENT_NAME] is None: | |||
| return None | |||
There was a problem hiding this comment.
I'm wondering why we did this choice at the time.
I would prefer to prefer to have {"intentName": null, "probability": 0.6} as for the other intents.
There was a problem hiding this comment.
Maybe this is the right time.
There was a problem hiding this comment.
I would also move this logic to the IntentClassifier since the logic of returning the most likely intents, something like:
class IntentClassifier(with_metaclass(ABCMeta, ProcessingUnit)):
@abstractmethod
def fit(self, dataset):
pass
def get_intent(self, text, intents_filter):
return self.get_intents(text)[0]
@abstractmethod
def get_intents(self, text):
"""Performs intent classification on the provided *text* and returns
the list of intents ordered by decreasing probability
The length of the returned list is exactly the number of intents in the
dataset + 1 for the None intent
.. note::
The probabilities returned along with each intent are not
guaranteed to sum to 1.0. They should be considered as scores
between 0 and 1.
"""
passsince I don't think the logic of returning the most likely intent out of the intents list will change from one IntentClassifier from another.
There was a problem hiding this comment.
But then you don't pass the intents_filter parameter.
|
When will there be a new release that includes this PR? |
|
Hi @timtutt a new release should be available by the end of the week or early next week |
Description:
This PR introduces 2 new parsing APIs.
The first one allows to run intent classification only, and get back a list of intents along with their probability:
The second API allows to extract the slots when the intent is already known:
On top of these two new APIs, an optional
top_nparameter is added to theparsemethod, allowing to perform intent parsing on thetop_nmost likely intents.This should address #623, and cover the feature brought by @deeiip in #715.
Checklist: