-
Notifications
You must be signed in to change notification settings - Fork 306
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
Add _validate_inputs
class method to constraints
#914
Conversation
Codecov Report
@@ Coverage Diff @@
## V1.0.0.dev #914 +/- ##
==============================================
+ Coverage 68.59% 70.73% +2.13%
==============================================
Files 38 39 +1
Lines 2866 3075 +209
==============================================
+ Hits 1966 2175 +209
Misses 900 900
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.
I think a lot of the classes can just use the base method if we make the base method use introspection to get the parameters names from __init__
106621f
to
72ba076
Compare
sdv/constraints/base.py
Outdated
if missing_values: | ||
if constraint_name == 'Inequality': | ||
errors.append(ValueError( | ||
f'Missing required values {missing_values} in an {constraint_name} constraint.' |
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.
I don't really like these if statements. @amontanez24 @pvk-developer what do you guys think of deleting it, so the error would just say a Inequality constraint
instead of an Inequality constraint
. It doesn't sound too wrong to me, and I'd rather the code be cleaner than a very minor grammatical thing.
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.
instead of doing the ifs we can just ake a variable ccalled article and use that in the f-string. eg.
article = 'an' if constraint_name == 'Inequality' else 'a'
errors.append(ValueError(f'Missing required values {missing_values} in {article} {constraint_name} constraint.'))
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.
I left a comment about removing the ifs for the Inequality
cases. Once that change is done this should be good to go!
* Add functionality and tests * Update tests * Switch tests to using regex * Trigger Build * Address feedback * Change Inequality if statements to one liner
* Add functionality and tests * Update tests * Switch tests to using regex * Trigger Build * Address feedback * Change Inequality if statements to one liner
* Add functionality and tests * Update tests * Switch tests to using regex * Trigger Build * Address feedback * Change Inequality if statements to one liner
* Add functionality and tests * Update tests * Switch tests to using regex * Trigger Build * Address feedback * Change Inequality if statements to one liner
* Add functionality and tests * Update tests * Switch tests to using regex * Trigger Build * Address feedback * Change Inequality if statements to one liner
* Add functionality and tests * Update tests * Switch tests to using regex * Trigger Build * Address feedback * Change Inequality if statements to one liner
* Add functionality and tests * Update tests * Switch tests to using regex * Trigger Build * Address feedback * Change Inequality if statements to one liner
Resolve #878.