Improve warnings: use color and sane defaults #4780

imLew · 2019-11-15T16:52:22Z

Proposed changes:

Improve warnings: use color and sane defaults when emitting warnings for our users
Makes sure the warning messages are colored and use a sane stack level for the warning message (e.g. by default this would be the line emitting the warning, which in most cases doesn't make sense).
If the code line is not relevant (e.g. if the warning is about an intent that should be listed in the domain but is not) we add a link to some relevant documentation page.
related to failing tests for expected warnings #4356

Status (please check what you already did):

made PR ready for code review
added some tests for the functionality
updated the documentation
updated the changelog
reformat files using black (please check Readme for instructions)

wochinge · 2019-12-16T15:18:14Z

@tmbo Who is taking this over?

tmbo · 2019-12-16T15:56:21Z

Can you please take that over as a squad?

I think it is unrealistic to set the warnings filter to error just yet (due to the strange errors we get with that), but here is what I think we should keep:

helper that sets default values for category and stacklevel
optional: red formatting of these messages in the log if possible

wochinge · 2019-12-16T17:04:57Z

You mean as production squad? ok

claassistantio · 2020-01-23T14:14:46Z

All committers have signed the CLA.

wochinge · 2020-01-24T12:50:39Z

@alwx I think @tmbo is currently also doing something here. Best keep off for now

tmbo · 2020-01-24T20:52:11Z

should have assigned me, sorry for the confusion

tmbo · 2020-01-28T13:52:40Z

rasa/core/tracker_store.py

@@ -722,9 +713,6 @@ def keys(self) -> Iterable[Text]:
    def retrieve(self, sender_id: Text) -> Optional[DialogueStateTracker]:
        """Create a tracker from all previously stored events."""

-        import sqlalchemy as sa


check if this import is needed

tmbo · 2020-01-28T13:53:49Z

rasa/nlu/classifiers/keyword_intent_classifier.py

+                intent_keyword_map = None
+            return cls(meta, intent_keyword_map)
+        else:
+            raise Exception(


converted this to an exception to stay in line with other components

tmbo · 2020-01-28T13:54:07Z

rasa/nlu/classifiers/sklearn_intent_classifier.py

@@ -108,7 +110,12 @@ def train(

            self.clf = self._create_classifier(num_threads, y)

-            self.clf.fit(X, y)
+            with warnings.catch_warnings():


ignore sklearn warnings

tmbo · 2020-01-28T13:54:15Z

rasa/nlu/classifiers/sklearn_intent_classifier.py

@@ -140,6 +147,7 @@ def _create_classifier(
            cv=cv_splits,
            scoring=self.component_config["scoring_function"],
            verbose=1,
+            iid=False,


prevents deprecation warning

tmbo · 2020-01-28T13:54:52Z

rasa/utils/common.py

@@ -295,3 +299,47 @@ def _lazyprop(self):
        return getattr(self, attr_name)

    return _lazyprop
+
+
+def raise_warning(


implementation for raise_warning

tmbo · 2020-01-28T13:55:36Z

setup.cfg

@@ -13,6 +13,7 @@ codestyle_exclude =
    rasa/core/policies/__init__.py
 filterwarnings =
    ignore::ResourceWarning:ruamel[.*]
+    #error


this should be activated later on to fail on warnings, doesn't work yet though

tmbo · 2020-01-28T13:56:21Z

tests/cli/test_utils.py



 def test_print_error_and_exit():
    with pytest.raises(SystemExit):
        rasa.cli.utils.print_error_and_exit("")
-
-
-def test_logging_capture(caplog: LogCaptureFixture):


tests removed since that function (assert_log_emitted ) isn't used anywhere

tmbo · 2020-01-28T13:58:52Z

This is ready for review and should go into the 1.7 release (cc @federicotdn).

@degiz can you please give this a review? it looks like a lot of changes but most of them are import reorg (thought about adding isort to prevent this from happening, but different story). I've commented on the lines where I did more than just rename warnings.warn.

tmbo · 2020-01-28T14:02:08Z

(@degiz ideally today or early tomorrow so that we can get it into the release - let me know if you have to much on your plate)

tmbo · 2020-01-28T14:02:46Z

data/test_domains/default_with_slots_and_no_actions.yml

@@ -18,7 +18,7 @@ slots:
  name:
    type: text

-templates:
+responses:


leftover from an earlier rename (created a warning that it is deprecated, that is why I changed it)

degiz

I've left few comments, but non of them is critical. So you can merge it the PR is urgent to get in.

degiz · 2020-01-29T12:04:42Z

rasa/core/agent.py

-        warnings.warn(
-            "Continue training will be removed in the next release. It won't be "
+        raise_warning(
+            "Continue training will be removed in the 2.0 release. It won't be "


optional: maybe it makes sense to mark the function as deprecated? is it exposed to the users?

mhm not sure what you mean, there isn't a @Deprecated like in java 😅

rasa/core/agent.py

rasa/core/brokers/file_producer.py

rasa/core/training/dsl.py

rasa/core/validator.py

rasa/nlu/classifiers/keyword_intent_classifier.py

rasa/nlu/featurizers/dense_featurizer/convert_featurizer.py

rasa/utils/common.py

tests/core/test_processor.py

ricwo · 2020-01-29T13:59:50Z

pytest has introduced coloured test summaries with 5.3.0: https://docs.pytest.org/en/latest/changelog.html#id29

we could upgrade to latest v 5.3.4

Co-Authored-By: Alexander Khizov <degiz@users.noreply.github.com>

tmbo · 2020-01-29T14:25:09Z

@ricwo that sounds great we should def do that, but would do that after the release 👍

…-testing

imLew mentioned this pull request Dec 13, 2019

failing tests for expected warnings #4356

Closed

alwx self-assigned this Jan 23, 2020

alwx removed their assignment Jan 24, 2020

tmbo self-assigned this Jan 24, 2020

tmbo changed the title ~~Improve testing~~ Improve warnings by coloring them and using sane defaults Jan 24, 2020

tmbo changed the title ~~Improve warnings by coloring them and using sane defaults~~ Improve warnings: use color and sane defaults Jan 24, 2020

tmbo reviewed Jan 28, 2020

View reviewed changes

tmbo added this to the Rasa 1.7 milestone Jan 28, 2020

tmbo assigned degiz and unassigned tmbo Jan 28, 2020

tmbo requested a review from degiz January 28, 2020 14:01

tmbo reviewed Jan 28, 2020

View reviewed changes

tmbo force-pushed the improve-testing branch 4 times, most recently from 211226e to 5a60ea5 Compare January 28, 2020 14:25

use colored warnings and provide doc links where helpful

d82d7e8

tmbo force-pushed the improve-testing branch from 5a60ea5 to d82d7e8 Compare January 28, 2020 14:26

fixed warning types for all brokers

32b272f

degiz approved these changes Jan 29, 2020

View reviewed changes

tmbo and others added 5 commits January 29, 2020 15:21

created constants for docs urls

c514799

Update rasa/core/validator.py

1b3d5d6

Co-Authored-By: Alexander Khizov <degiz@users.noreply.github.com>

Update rasa/nlu/classifiers/keyword_intent_classifier.py

a3d55e0

Co-Authored-By: Alexander Khizov <degiz@users.noreply.github.com>

Update rasa/nlu/classifiers/keyword_intent_classifier.py

e726f1f

Co-Authored-By: Alexander Khizov <degiz@users.noreply.github.com>

Update rasa/nlu/featurizers/dense_featurizer/convert_featurizer.py

89fab67

Co-Authored-By: Alexander Khizov <degiz@users.noreply.github.com>

tmbo added 2 commits January 29, 2020 15:27

added types

081a283

Merge branch 'improve-testing' of github.com:RasaHQ/rasa into improve…

7322c41

…-testing

tmbo merged commit d53eef5 into master Jan 29, 2020

tmbo deleted the improve-testing branch January 29, 2020 14:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve warnings: use color and sane defaults #4780

Improve warnings: use color and sane defaults #4780

imLew commented Nov 15, 2019 •

edited by tmbo

wochinge commented Dec 16, 2019

tmbo commented Dec 16, 2019

wochinge commented Dec 16, 2019

claassistantio commented Jan 23, 2020 •

edited

wochinge commented Jan 24, 2020

tmbo commented Jan 24, 2020

tmbo Jan 28, 2020

tmbo Jan 28, 2020

tmbo Jan 28, 2020

tmbo Jan 28, 2020

tmbo Jan 28, 2020

tmbo Jan 28, 2020

tmbo Jan 28, 2020

tmbo commented Jan 28, 2020

tmbo commented Jan 28, 2020

tmbo Jan 28, 2020

degiz left a comment

degiz Jan 29, 2020

tmbo Jan 29, 2020

ricwo commented Jan 29, 2020

tmbo commented Jan 29, 2020

Improve warnings: use color and sane defaults #4780

Improve warnings: use color and sane defaults #4780

Conversation

imLew commented Nov 15, 2019 • edited by tmbo

wochinge commented Dec 16, 2019

tmbo commented Dec 16, 2019

wochinge commented Dec 16, 2019

claassistantio commented Jan 23, 2020 • edited

wochinge commented Jan 24, 2020

tmbo commented Jan 24, 2020

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

tmbo commented Jan 28, 2020

tmbo commented Jan 28, 2020

Choose a reason for hiding this comment

degiz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ricwo commented Jan 29, 2020

tmbo commented Jan 29, 2020

imLew commented Nov 15, 2019 •

edited by tmbo

claassistantio commented Jan 23, 2020 •

edited