Skip to content
This repository has been archived by the owner on Aug 22, 2019. It is now read-only.

Extract message intent instead of setting the node with a dict when using visualization #1883

Merged
merged 2 commits into from Apr 3, 2019

Conversation

MetcalfeTom
Copy link
Contributor

@MetcalfeTom MetcalfeTom commented Apr 2, 2019

Fixes RasaHQ/rasa#2995

Under certain circumstances we were passing the message parse data to the node label during interactive learning visualization, which (since this is a dictionary) was causing fatal errors.

Proposed changes:

  • Use message intent name instead of the parse data as the graph node label

Status (please check what you already did):

  • made PR ready for code review
  • updated the changelog

@codeclimate
Copy link

codeclimate bot commented Apr 2, 2019

Code Climate has analyzed commit 1386fa8 and detected 0 issues on this pull request.

View more on Code Climate.

@MetcalfeTom MetcalfeTom changed the base branch from master to 0.13.x April 2, 2019 08:14
@MetcalfeTom MetcalfeTom changed the title Extract message intent instead of setting the node with a dict Extract message intent instead of setting the node with a dict when using visualization Apr 2, 2019
Copy link
Contributor

@paulaWesselmann paulaWesselmann left a comment

Choose a reason for hiding this comment

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

Why is message set to parse_data here?

            if isinstance(el, UserUttered):
                if not el.intent:
                    message = interpreter.parse(el.text)
                else:
                    message = el.parse_data

@MetcalfeTom
Copy link
Contributor Author

@paulaWesselmann

Just a convenient way of passing both the text and the intent; keeping the text makes the graph readable and keeping the intent allows us to compare equivalent nodes. If the same intent has appeared twice, we overwrite the node label with the subsequent message text.

@paulaWesselmann
Copy link
Contributor

Approved because it error proofs this from happening. But I'm still confused on how it comes to this sequence of events that it is able to happen at all.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants