-
Notifications
You must be signed in to change notification settings - Fork 22
feat: generic validator for seperator and type filter #30
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
Conversation
filters/tests/test_validations.py
Outdated
| INT, LONG | ||
| ] | ||
|
|
||
| class GenericSeperatedValidatorTestCase(unittest.TestCase): |
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.
Spelling error: Use Separated everywhere.
filters/validations.py
Outdated
| GenericSeperatedValidator.validation() should be passed to the schema | ||
| >>> CSVofIntegers = GenericSeperatedValidator(int, ',') | ||
| >>> CSVofIntegers.validation()('1,2,3') |
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 naming the method validation, you can override the __call__ method so that this validator complies with the others.
filters/validations.py
Outdated
| return values | ||
| else: | ||
| raise ValueError | ||
| except ValueError as e: |
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.
You can use a common except block for ValueError and Invalid
filters/validations.py
Outdated
| ''' | ||
|
|
||
| def __init__(self, validation_function, seperator=','): | ||
| if type(seperator) != basestring: |
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.
Use isinstance(separator, basestring). basestring is an abstract superclass of str and unicode so this will return True for all string values.
filters/validations.py
Outdated
| ''' | ||
|
|
||
| def __init__(self, validation_function, separator=','): | ||
| if isinstance(separator, basestring): |
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.
This should be if not isinstance(separator, basestring). Maybe add a test for this case?
filters/validations.py
Outdated
| else: | ||
| self.value_type = validation_function.__name__ | ||
|
|
||
| def __call__(self, msg=None): |
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.
You can move the msg argument to __init__ and make __call__ a normal function that takes the value (instead of it being an HOF). That way, it'll be completely consistent with the other validators, except that it takes arguments in addition to msg.
|
There are some commits which aren't worded properly or are fix-up commits. When you're done with your changes, please reword your commits wherever you feel you should and squash fix-up commits. You can refer to this for good commit message practices. |
d07db3a to
c699bdc
Compare
|
updated |
No description provided.