-
Notifications
You must be signed in to change notification settings - Fork 31
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
Vladiate enhancements #62
base: master
Are you sure you want to change the base?
Conversation
… of inferring them from CSV header.
@mmuru Can you get the CI passing? Looks like you'll need to add |
@di: Added python-dateutil as a dependency and fixed flake8 lint errors. CI passed for python 2.7 but not for other python 3.x versions. I don't have a python 3 environment so any help is appreciated. |
vladiate/inputs.py
Outdated
@@ -31,8 +32,13 @@ def __init__(self, filename): | |||
self.filename = filename | |||
|
|||
def open(self): | |||
with open(self.filename, 'r') as f: | |||
return f.readlines() | |||
with open(self.filename, 'rb') as f: |
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.
@mmuru I think the current test failure is because you changed the mode here from r
to rb
. The check for .gz
extension should probably happen before trying to open the file.
@mmuru Are you able to get this PR passing? Or shall we close it? |
I'll work on this PR week and provide an update EOW.
… On Mar 18, 2019, at 8:19 AM, Dustin Ingram ***@***.***> wrote:
@mmuru Are you able to get this PR passing? Or shall we close it?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
@di: Now this PR is passing and let me know if you have any questions otherwise merge it. |
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 needs documentation and tests for the new validators, and details on how to use the new arguments for S3File
and Vlad
.
Also, if you can add a |
This PR extends the current Vladiate to support our CSV file validation requirements.
The following enhancements have been added