-
Notifications
You must be signed in to change notification settings - Fork 4
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
fix/none_type #27
fix/none_type #27
Conversation
closes #26
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not make the default an empty dict instead of None and then change it later?
setting a empty dict or list in the method definition is a common mistake in python Default values in Python are created exactly once, when the function is defined. If that object is changed, subsequent calls to the function will refer to the changed object, ie, context would be kept around across calls eg
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Python really needs an immutable dict and list type... :P
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests are failing
they have been for a while :P unrelated to this PR we really need to do some cleanup around all skill repos, many have copy pasted automations just to get them published but then have failing tests that just came from somewhere else |
closes #26