Skip to content
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

optional slot mappings #7171

Merged
merged 12 commits into from
Nov 6, 2020
Merged

optional slot mappings #7171

merged 12 commits into from
Nov 6, 2020

Conversation

wochinge
Copy link
Contributor

@wochinge wochinge commented Nov 3, 2020

Proposed changes:

  • fix Allow forms without default slot mappings / only custom slots mappings #7068
  • forms do not have to have slot mappings any more. They can be empty instead which allows moving all slot mappings to the custom validate action.
  • the usage of the Rasa Open Source 2 FormAction can be overridden by defining a custom action with the name of the form. This can e.g. be used to use the deprecated FormAction from the Rasa SDK.
  • When a Domain defines forms as a list of strings (Rasa Open Source 1 format) then it's assumed that the user wants to use the deprecated FormAction as well. A warning is printed.

Status (please check what you already did):

  • added some tests for the functionality
  • updated the documentation
  • updated the changelog (please check changelog for instructions)
  • reformat files using black (please check Readme for instructions)

@wochinge wochinge marked this pull request as ready for review November 3, 2020 17:00
@wochinge wochinge requested review from a team, federicotdn and Ghostvv and removed request for a team November 3, 2020 17:00
@Ghostvv
Copy link
Contributor

Ghostvv commented Nov 4, 2020

what is the difference between defining forms as list of strings vs list of dicts with no values?

@wochinge
Copy link
Contributor Author

wochinge commented Nov 4, 2020

what is the difference between defining forms as list of strings vs list of dicts with no values?

The list format is the old format for the forms: section which was used in Rasa Open Source.
Rasa Open Source expects the forms: section to be provided as Dict

@Ghostvv
Copy link
Contributor

Ghostvv commented Nov 4, 2020

how does it look from user point of view in domain file? Should we document that?

@wochinge
Copy link
Contributor Author

wochinge commented Nov 4, 2020

how does it look from user point of view in domain file? Should we document that?

What exactly do you mean? I don't think we should document an outdated format. Handling that is just a failsafe for users who haven't migrated their domain format yet. I can add another section to the documentation which describes how they can use the old SDK FormAction with a bunch of warnings 👍

@Ghostvv
Copy link
Contributor

Ghostvv commented Nov 4, 2020

what I mean is. If I were to define a bunch of forms without slot mappings, I'd put them in domain file similar as actions. How would it look different if I need to define them as a list of dicts

@wochinge
Copy link
Contributor Author

wochinge commented Nov 4, 2020

f I were to define a bunch of forms without slot mappings, I'd put them in domain file similar as actions. How would it look different if I need to define them as a list of dicts

I made an example of forms without slot mappings in the changelog. It's simply as you would usually define forms - just that you leave apart the slot mappings. The old format within Rasa Open Source 1 was:

forms:
- form1
- form2

@Ghostvv
Copy link
Contributor

Ghostvv commented Nov 4, 2020

now I'm confused, sorry 🙈 but then it is a list of strings. But I'd like to use 2.0 forms

@wochinge
Copy link
Contributor Author

wochinge commented Nov 4, 2020

That's a Rasa Open Source 2.0 forms without slot mapping

forms:
  my_form:
    # no slot mappings

@akelad updated the migration guide with instructions

@Ghostvv
Copy link
Contributor

Ghostvv commented Nov 4, 2020

so

forms:
  form1:
  form2:

or do I have to add a comment?

@wochinge
Copy link
Contributor Author

wochinge commented Nov 4, 2020

or do I have to add a comment?

No comment needed

@Ghostvv
Copy link
Contributor

Ghostvv commented Nov 4, 2020

that is very weird format to me, but ok

Copy link
Contributor

@federicotdn federicotdn left a comment

Choose a reason for hiding this comment

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

👍 👍

Comment on lines +604 to +610
"The `forms` section in the domain used the old Rasa Open Source 1 "
"list format to define forms. Rasa Open Source will be configured to "
"use the deprecated `FormAction` within the Rasa SDK. If you want to "
"use the new Rasa Open Source 2 `FormAction` adapt your `forms` "
"section as described in the documentation. Support for the "
"deprecated `FormAction` in the Rasa SDK will be removed in Rasa Open "
"Source 3.0.",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"The `forms` section in the domain used the old Rasa Open Source 1 "
"list format to define forms. Rasa Open Source will be configured to "
"use the deprecated `FormAction` within the Rasa SDK. If you want to "
"use the new Rasa Open Source 2 `FormAction` adapt your `forms` "
"section as described in the documentation. Support for the "
"deprecated `FormAction` in the Rasa SDK will be removed in Rasa Open "
"Source 3.0.",
"The `forms` section in the domain used the old Rasa Open Source 1.0 "
"list format to define forms. Rasa Open Source will be configured to "
"use the deprecated `FormAction` within the Rasa SDK. If you want to "
"use the new Rasa Open Source 2.0 `FormAction` adapt your `forms` "
"section as described in the documentation. Support for the "
"deprecated `FormAction` in the Rasa SDK will be removed in Rasa Open "
"Source 3.0.",

Copy link
Contributor Author

Choose a reason for hiding this comment

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

why do we need the .0? It was not only part of Rasa Open Source 1.0, but of all Rasa Open Source 1.x versions, right? Same for 2.0: It's part of 2.x

is_form = name in domain.form_names
# Users can override the form by defining an action with the same name as the form
user_overrode_form_action = is_form and name in domain.user_actions
if is_form and not user_overrode_form_action:
Copy link
Contributor

Choose a reason for hiding this comment

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

    if is_form and not user_overrode_form_action:

is equivalent to (X and not (X and Y)):

    if is_form and not (is_form and name in domain.user_actions):

which in turn is equivalent to (X and not Y):

    if is_form and name not in domain.user_actions:

So then maybe we could just write:

    if name in domain.form_names and name not in domain.user_actions:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I assigned names to the single checks to make their meaning more explicit.

Developers might wonder what name not in domain.user_actions means while user_overrode_form_action conveys way more information about this. What do you think?

@wochinge wochinge requested a review from indam23 November 4, 2020 17:12
@wochinge
Copy link
Contributor Author

wochinge commented Nov 4, 2020

@melindaloubser1 Could you please have a look at the docs?

Copy link
Contributor

@indam23 indam23 left a comment

Choose a reason for hiding this comment

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

Just some phrasing changes

docs/docs/default-actions.mdx Outdated Show resolved Hide resolved
docs/docs/migration-guide.mdx Outdated Show resolved Hide resolved
docs/docs/migration-guide.mdx Outdated Show resolved Hide resolved
docs/docs/default-actions.mdx Outdated Show resolved Hide resolved
Co-authored-by: Melinda Loubser <32034278+melindaloubser1@users.noreply.github.com>
@wochinge
Copy link
Contributor Author

wochinge commented Nov 6, 2020

@Ghostvv @Ghostvv Do you want to have another look or can I merge?

Copy link
Contributor

@Ghostvv Ghostvv left a comment

Choose a reason for hiding this comment

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

I don't think the format:

forms:
  form1:
  form2:

is intuitive or clear, the need or no need of - in yaml is a mess. Otherwise looks good to me

@rasabot rasabot merged commit f2b05c4 into master Nov 6, 2020
@rasabot rasabot deleted the optional-mappings branch November 6, 2020 12:41
@wochinge wochinge mentioned this pull request Nov 20, 2020
4 tasks
@SuunnyyGoo
Copy link

Hello Everybody,

First thanks for your job, Rasa is a great toolbox, it helps us a lot !

I am facing the current issue and i dont found it clear, i'm using dist 0.37.0 | 2.3.1 | 2.3.1 with gke rasa-x-helm install.

I have a form used to meet the user, at first I started with slot mapping from the domain, but looking for more accuracy, I created a validate_form class in my custom action server to improve slot extraction. One by one I finnally added all my slots mappings in my custom action but now the form is still called but not effective anymore.
I read this topic and the linked ones but i'm still confused about the use of the FormAction which is said as a way to manage depreciation, is that case one sort of depreciation ? Because i don't want te create a whole FormAction, i just want to find out my validate_form class

So I tried to implement a FormAction inherited from Rasa Core with only the name and the required slots (keeping the validate_form class nearby, this FormAction might call it ?..), but it is not registred when starting the action server and my form is achieved right after starting (without asking questions).

So I tried to implement a FormAction from Rasa_sdk (which is commented as deprecated) with overides for the name, the required slots and the submit methods. Actions my_form and validate_my_form are well reguistred ! But when my form is accessed it isn't able to use the nlu extraction and always run in fallback, stucking on 1st question or getting out the form.

My domain contains :

actions:
- my_form
- validate_my_form
forms:
  my_form: null
  other_form:
    slot:(...)

The "null" is added by Rasa X.

How do I reach validate_my_form when there are no commun mapping ? Which FormAction must i use to explicit the form's slots and access the validate_form (inside action server, i guess the core's one, but i only achieve register sdk's one).

Do i have to make my class inherit from both FormAction and FormValidationAction ?

Thank you a lot for reading, let's give a nudge to your karma by helping me ! :D

@wochinge
Copy link
Contributor Author

wochinge commented Mar 5, 2021

@SuunnyyGoo Please ask usage questions in Rasa forum

@travelmail26
Copy link

Hi, rasa newbie here. Is there an example of what this looks like somewhere? thanks!

@indam23
Copy link
Contributor

indam23 commented Mar 15, 2021

@travelmail26 Please ask usage questions in Rasa forum.
You can also look at these docs

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

Successfully merging this pull request may close these issues.

Allow forms without default slot mappings / only custom slots mappings
8 participants