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.

Sign up for GitHub

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

Jump to bottom

fixed mypy operator error type #9062

Merged
merged 31 commits into from
Jul 27, 2021
Merged

fixed mypy operator error type #9062

merged 31 commits into from
Jul 27, 2021

Conversation

twerkmeister
Copy link
Contributor

@twerkmeister twerkmeister commented Jul 7, 2021 •
edited
Loading

Proposed changes:

Status (please check what you already did):

  • enable mypy checks for operator
  • all existing operator issues are fixed

@twerkmeister twerkmeister requested a review from a team as a code owner July 7, 2021 10:46
@twerkmeister twerkmeister requested review from a team and koernerfelicia and removed request for a team July 7, 2021 10:46
@wochinge wochinge requested review from wochinge and removed request for a team July 12, 2021 08:09
wochinge
wochinge reviewed Jul 12, 2021
View reviewed changes
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.

Thanks for fixing so many mypy errors 🎉

twerkmeister and others added 19 commits July 12, 2021 14:18
@twerkmeister
using guards to increase readability
0d80dca
@twerkmeister
set properly typed defaults
f1946fa
@twerkmeister
using f strings
73e4b3d
@twerkmeister @wochinge
Update rasa/utils/endpoints.py
bfddc3c 
Co-authored-by: Tobias Wochinger <t.wochinger@rasa.com>
@twerkmeister @wochinge
Update rasa/nlu/featurizers/sparse_featurizer/count_vectors_featurize…
b0f7929 
…r.py

Co-authored-by: Tobias Wochinger <t.wochinger@rasa.com>
@twerkmeister
Added default parameter for max_relative_position
b54f901
@twerkmeister
Merge branch '9058-mypy-operator' of github.com:RasaHQ/rasa into 9058…
95347fb 
…-mypy-operator
@twerkmeister
changed default endpoint to empty str
bcf7061
@twerkmeister
Turned to simpler changes in count vectors featurizer
d89e962
@twerkmeister
Code quality
b7dece2
@twerkmeister
Fixed tests
d6e8a53
@twerkmeister
Fixed tests and code quality
7c73ceb
@twerkmeister
Added guard instead of endpoints url as empty string
bb8d23d
@twerkmeister
Fixed doc string code quality
b9eec04
@twerkmeister
Merge branch 'main' into 9058-mypy-operator
6e79248
@twerkmeister
Added corrupt metadata exception
a96ad4b
@twerkmeister
removed unused imports
8030180
@twerkmeister
Merge branch '9058-mypy-operator' of github.com:RasaHQ/rasa into 9058…
3ad1ac4 
…-mypy-operator
@twerkmeister
just a empty commit test
97cae31
wochinge
wochinge reviewed Jul 14, 2021
View reviewed changes
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.

Looking very good and also simplifies the code! 💯

@koernerfelicia
Copy link
Contributor

Hi @twerkmeister, got back today so am just looking at this for the first time now. Could you let me know how I would check that the warnings for operator are all gone? I ran make types but this raises no issues here or on main.

@wochinge
Copy link
Contributor

I ran make types but this raises no issues here or on main.

operator issues are suppressed on main and hence raise no error there (you can edit remove it from the ignored section in setup.cfg as Thomas has done in this PR) to see all errors.

And I guess they are fixed on this branch which is why you can't see them here.

koernerfelicia
koernerfelicia reviewed Jul 16, 2021
View reviewed changes
Copy link
Contributor

@koernerfelicia koernerfelicia left a comment

Choose a reason for hiding this comment

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

Love the cleanup 😍 Had a few small comments, otherwise looks quite good!

@twerkmeister
Copy link
Contributor Author

Love the cleanup 😍 Had a few small comments, otherwise looks quite good!

Thanks a lot for the comments :)

@twerkmeister twerkmeister mentioned this pull request Jul 20, 2021
Closed
4 tasks
@m-vdb
Copy link
Collaborator

m-vdb commented Jul 23, 2021

@twerkmeister is this one ready for re-review? not sure if you know, you can re-request reviews by clicking on that icon 🔁
Screenshot 2021-07-23 at 11 36 26

wochinge
wochinge approved these changes Jul 26, 2021
View reviewed changes
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.

🚀

@twerkmeister twerkmeister merged commit f78a6a1 into main Jul 27, 2021
@twerkmeister twerkmeister deleted the 9058-mypy-operator branch July 27, 2021 12:03
ErickGiffoni pushed a commit to FGA-GCES/rasa that referenced this pull request Sep 9, 2021
@twerkmeister @ErickGiffoni
fixed mypy operator error type (RasaHQ#9062)
ed117c2 
* fixed mypy operator error type
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@koernerfelicia koernerfelicia koernerfelicia left review comments

@wochinge wochinge wochinge approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

mypy: Fix operator
4 participants
@twerkmeister @koernerfelicia @wochinge @m-vdb