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

unfortunate use of kwarg 'kwargs' results in some arguments not having effect #4458

Closed
imLew opened this issue Sep 13, 2019 · 2 comments · Fixed by #5063
Closed

unfortunate use of kwarg 'kwargs' results in some arguments not having effect #4458

imLew opened this issue Sep 13, 2019 · 2 comments · Fixed by #5063
Assignees
Labels
area:rasa-oss 🎡 Anything related to the open source Rasa framework difficulty:easy 🦋 help wanted type:bug 🐛 Inconsistencies or issues which will cause an issue or problem for users or implementors. type:maintenance 🔧 Improvements to tooling, testing, deployments, infrastructure, code style.

Comments

@imLew
Copy link
Contributor

imLew commented Sep 13, 2019

In a couple of places (e.g. rasa.train and rasa.core.train) we have a keyword argument called kwargs.
This is a confusing name as **kwargs is conventionally used to capture 0 or more additional, optional keyword arguments.
These function signatures should be changed, either by using a different name for this variable (additional_arguments) or by adapting them to use **kwargs.

This also causes the argument dump_stories of rasa.train to have no effect at all. (See https://forum.rasa.com/t/how-can-i-use-dump-stories-from-script/15433)

Update:
Another place where we use kwargs as a name for a dict instead of a container for optional keyword arguments is

def test(

@imLew imLew added type:maintenance 🔧 Improvements to tooling, testing, deployments, infrastructure, code style. difficulty:easy 🦋 area:rasa-oss 🎡 Anything related to the open source Rasa framework type:bug 🐛 Inconsistencies or issues which will cause an issue or problem for users or implementors. labels Sep 13, 2019
@imLew imLew changed the title unfortunate use of kwarg 'kwargs' unfortunate use of kwarg 'kwargs' results in some arguments not having effect Sep 19, 2019
@imLew imLew added type:maintenance 🔧 Improvements to tooling, testing, deployments, infrastructure, code style. difficulty:easy 🦋 and removed type:maintenance 🔧 Improvements to tooling, testing, deployments, infrastructure, code style. difficulty:easy 🦋 labels Sep 19, 2019
@AlvaroMonteagudo
Copy link

Any thoughts on this, it's been a while

@imLew
Copy link
Contributor Author

imLew commented Nov 15, 2019

I'm sorry @AlvaroMonteagudo, we haven't had time to address this yet.
Were you not able to fix it locally?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:rasa-oss 🎡 Anything related to the open source Rasa framework difficulty:easy 🦋 help wanted type:bug 🐛 Inconsistencies or issues which will cause an issue or problem for users or implementors. type:maintenance 🔧 Improvements to tooling, testing, deployments, infrastructure, code style.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants