-
Notifications
You must be signed in to change notification settings - Fork 37
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
Replace eval by getattr #84
Conversation
Codecov Report
@@ Coverage Diff @@
## master #84 +/- ##
==========================================
+ Coverage 95.09% 95.33% +0.23%
==========================================
Files 7 7
Lines 1284 1285 +1
Branches 115 116 +1
==========================================
+ Hits 1221 1225 +4
+ Misses 43 41 -2
+ Partials 20 19 -1
Continue to review full report at Codecov.
|
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.
Please revert the deletion of the guard which checks for the argument length.
Please rebase/merge master once #93 is merged. |
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.
Thanks for your patience, @eumiro - this PR is really special :-)
Ok, this looks good now! Thanks for your patience. There is one last thing... Could you please add a docstring to both For reference, I like this short blog post on how to write good docstrings for tests: Thanks a lot! |
The deed is done. |
No need to use
eval
here.