-
Notifications
You must be signed in to change notification settings - Fork 212
fix: set inputs as optional #109
fix: set inputs as optional #109
Conversation
Codecov Report
@@ Coverage Diff @@
## master #109 +/- ##
==========================================
+ Coverage 86.34% 86.38% +0.03%
==========================================
Files 47 47
Lines 1509 1513 +4
==========================================
+ Hits 1303 1307 +4
Misses 206 206
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
@aniketmaurya Could you add a test as well for this edge case? It should be able to raise an Exception. |
Ok will do |
…lightning-flash into optional_input_108
Usually it's frowned upon to have empty lists/tuples as default arguments, see: https://nikos7am.com/posts/mutable-default-arguments/ I would make the default values |
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.
Awesome thanks :)
What does this PR do?
categorical_inputs
andnumerical_inputs
are made optional. The error will raise only when both are None.Fixes #108
Before submitting
PR review
Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.
Did you have fun?
Make sure you had fun coding 🙃