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

Switch to mypy part1 #6511

Merged
merged 53 commits into from
Mar 16, 2021
Merged

Switch to mypy part1 #6511

merged 53 commits into from
Mar 16, 2021

Conversation

m-vdb
Copy link
Collaborator

@m-vdb m-vdb commented Aug 31, 2020

Proposed changes:
Fix the following typing issues:

  • dict-item
  • list-item
  • call-arg

Leftovers to merge this PR

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)

@m-vdb m-vdb mentioned this pull request Sep 1, 2020
@m-vdb
Copy link
Collaborator Author

m-vdb commented Oct 12, 2020

@wochinge after merging master into this branch, I have to fix more issues. I have a question about ActionExecuted. Is it safe to assume that it should only be initialised with action_name and/or action_text, but if both are None, then we should raise an error?

Copy link
Contributor

@wochinge wochinge left a comment

Choose a reason for hiding this comment

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

All done besides from a small snippet in rasa.server.

Nice work! Cool to see that mypy also helps us detecting and removing technical debt! 🥇

pyproject.toml Show resolved Hide resolved
rasa/cli/train.py Outdated Show resolved Hide resolved
rasa/core/actions/action.py Outdated Show resolved Hide resolved
rasa/core/actions/forms.py Show resolved Hide resolved
rasa/core/actions/two_stage_fallback.py Outdated Show resolved Hide resolved
rasa/nlu/model.py Show resolved Hide resolved
tests/cli/test_rasa_interactive.py Outdated Show resolved Hide resolved
rasa/server.py Show resolved Hide resolved
rasa/server.py Outdated Show resolved Hide resolved
@m-vdb
Copy link
Collaborator Author

m-vdb commented Oct 14, 2020

@wochinge thanks for your review! This PR was not really ready for review, but the "part0" is. Would you mind taking a look?

@wochinge
Copy link
Contributor

🙈 will do

Base automatically changed from switch-to-mypy to master October 19, 2020 09:16
@m-vdb m-vdb requested a review from a team as a code owner November 11, 2020 14:21
rasa/cli/train.py Outdated Show resolved Hide resolved
rasa/shared/core/trackers.py Outdated Show resolved Hide resolved
rasa/shared/core/training_data/structures.py Show resolved Hide resolved
rasa/core/channels/rasa_chat.py Outdated Show resolved Hide resolved
@m-vdb m-vdb requested a review from wochinge March 16, 2021 08:52
Copy link
Contributor

@wochinge wochinge left a comment

Choose a reason for hiding this comment

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

Nice to get this in! 🚀 Great work! Do we have follow up issues for the enable squad for this?

@m-vdb
Copy link
Collaborator Author

m-vdb commented Mar 16, 2021

@wochinge no we don't as I don't know how the squad wants to split the work on this yet. Maybe let's have a single placeholder issue to discuss this, and create other issues when we're ready to tackle it?

@m-vdb m-vdb enabled auto-merge March 16, 2021 09:23
@wochinge
Copy link
Contributor

Do you have some up do date statistics how often each error type is occuring? How about putting this in the squad channel? In my opinion we should have ready to go issue for refinement and then sprint planning.

@m-vdb
Copy link
Collaborator Author

m-vdb commented Mar 16, 2021

let me compile this and share it

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.

None yet

2 participants