Skip to content
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

Last touches #1874

Merged
merged 4 commits into from Jul 2, 2016
Merged

Conversation

antgonza
Copy link
Member

Still missing the timestamp fix but want to see how the tests go ...

@antgonza
Copy link
Member Author

Ready for review: @ElDeveloper and @josenavas

@@ -1359,10 +1359,27 @@ def validate(self, restriction_dict):
if datatype == datetime:
val = str(val)
try:
datetime.strptime(val, '%m/%d/%y %H:%M:%S')
datetime.strptime(val, '%m/%d/%Y %H:%M:%S')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Argh! That hurts! So sad that datetime doesn't support the "or" operator. What about something like this?

formats = ['%m/%d/%Y %H:%M:%S', '%m/%d/%Y %H:%M', '%m/%d/%Y %H', '%m/%d/%Y', '%m/%Y', '%Y']
dt = None
for fmt in formats:
    try:
        dt = datetime.strptime(val, fmt)
    except ValueError:
        pass
if dt is None:
    warning_msg.append(....)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😱

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like it, will change.

@josenavas
Copy link
Contributor

Thanks @antgonza few comments

@ElDeveloper
Copy link
Member

👍

@josenavas
Copy link
Contributor

👍 given tests pass

@josenavas josenavas changed the title WIP: Last touches Last touches Jun 30, 2016
@josenavas
Copy link
Contributor

@antgonza there are a couple of real errors in here. Should be easy to fix though.

@antgonza
Copy link
Member Author

antgonza commented Jul 1, 2016

Thanks, just pushed the fixes.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling b47e486 on antgonza:last-touches into * on biocore:str-ing_info_fles*.

@josenavas josenavas merged commit f9bfa76 into qiita-spots:str-ing_info_fles Jul 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants