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 support for ISO 8601 formatting to DateTime field #345

Merged
merged 6 commits into from Nov 21, 2014

Conversation

Projects
None yet
2 participants
@Ch00k
Contributor

Ch00k commented Nov 21, 2014

No description provided.

@Ch00k

This comment has been minimized.

Show comment
Hide comment
@Ch00k

Ch00k Nov 21, 2014

Contributor

Looks like this still needs more work: implement datetime_from_iso8601, add more unittests (and fix the failing one).

Contributor

Ch00k commented Nov 21, 2014

Looks like this still needs more work: implement datetime_from_iso8601, add more unittests (and fix the failing one).

@joshfriend

This comment has been minimized.

Show comment
Hide comment
@joshfriend

joshfriend Nov 21, 2014

can we just call this argument format?

joshfriend commented on flask_restful/fields.py in 8c435cf Nov 21, 2014

can we just call this argument format?

This comment has been minimized.

Show comment
Hide comment
@Ch00k

Ch00k Nov 21, 2014

Owner

We can't because then it conflicts with the format method.

Owner

Ch00k replied Nov 21, 2014

We can't because then it conflicts with the format method.

This comment has been minimized.

Show comment
Hide comment
@joshfriend

joshfriend Nov 21, 2014

ah, good point.

joshfriend replied Nov 21, 2014

ah, good point.

@joshfriend

This comment has been minimized.

Show comment
Hide comment
@joshfriend

joshfriend Nov 21, 2014

What if you assume they passed a valid date format string and use strftime to try to format it using the string passed?

joshfriend commented on flask_restful/fields.py in 8c435cf Nov 21, 2014

What if you assume they passed a valid date format string and use strftime to try to format it using the string passed?

This comment has been minimized.

Show comment
Hide comment
@Ch00k

Ch00k Nov 21, 2014

Owner

Sorry, I'm not sure I understand what you mean. Could you please explain a little bit more on this?

Owner

Ch00k replied Nov 21, 2014

Sorry, I'm not sure I understand what you mean. Could you please explain a little bit more on this?

This comment has been minimized.

Show comment
Hide comment
@joshfriend

joshfriend Nov 21, 2014

This would let people specify a custom datetime format using the standard python time string format.

>>> d = datetime.now()
>>> d.strftime("%d/%m/%y")
'11/21/14'

joshfriend replied Nov 21, 2014

This would let people specify a custom datetime format using the standard python time string format.

>>> d = datetime.now()
>>> d.strftime("%d/%m/%y")
'11/21/14'

This comment has been minimized.

Show comment
Hide comment
@Ch00k

Ch00k Nov 21, 2014

Owner

If I understand this right, this is just adding more flexibility which is not quite related to this PR. Maybe file that as a feature request and implement separately? Or am I still missing your point?

Owner

Ch00k replied Nov 21, 2014

If I understand this right, this is just adding more flexibility which is not quite related to this PR. Maybe file that as a feature request and implement separately? Or am I still missing your point?

This comment has been minimized.

Show comment
Hide comment
@joshfriend

joshfriend Nov 21, 2014

Sure, that could be a separate feature request since it isn't directly related to what you created this PR for. This should be fine as is.

joshfriend replied Nov 21, 2014

Sure, that could be a separate feature request since it isn't directly related to what you created this PR for. This should be fine as is.

@joshfriend

This comment has been minimized.

Show comment
Hide comment
@joshfriend

joshfriend Nov 21, 2014

We have a inputs.iso8601interval right now, maybe rename this to iso8601datetime to make it more consistent?

joshfriend commented on flask_restful/inputs.py in 51e7185 Nov 21, 2014

We have a inputs.iso8601interval right now, maybe rename this to iso8601datetime to make it more consistent?

This comment has been minimized.

Show comment
Hide comment
@Ch00k

Ch00k Nov 21, 2014

Owner

I was trying to make this function's naming consistent with datetime_from_rfc822. I don't think renaming it to what you suggest is a good idea, tbh.

Owner

Ch00k replied Nov 21, 2014

I was trying to make this function's naming consistent with datetime_from_rfc822. I don't think renaming it to what you suggest is a good idea, tbh.

@joshfriend

This comment has been minimized.

Show comment
Hide comment
@joshfriend

joshfriend Nov 21, 2014

What was your motivation for removing pytz? I've been using it in tests and it works fine.

joshfriend commented on 8c0fe81 Nov 21, 2014

What was your motivation for removing pytz? I've been using it in tests and it works fine.

This comment has been minimized.

Show comment
Hide comment
@Ch00k

Ch00k Nov 21, 2014

Owner

In this test suite we define a class called UTC, use it in just two tests and then use pytz in all the other tests. I thought I'll just get rid of pytz and use UTC everywhere else, too. Of course as another option we could remove the UTC class and use pytz.UTC eveywhere.

Owner

Ch00k replied Nov 21, 2014

In this test suite we define a class called UTC, use it in just two tests and then use pytz in all the other tests. I thought I'll just get rid of pytz and use UTC everywhere else, too. Of course as another option we could remove the UTC class and use pytz.UTC eveywhere.

This comment has been minimized.

Show comment
Hide comment
@joshfriend

joshfriend Nov 21, 2014

other sections of the inputs module still require pytz (_normalize_interval). since it's already installed/required, we might as well use it.

joshfriend replied Nov 21, 2014

other sections of the inputs module still require pytz (_normalize_interval). since it's already installed/required, we might as well use it.

This comment has been minimized.

Show comment
Hide comment
@Ch00k

Ch00k Nov 21, 2014

Owner

Yes, that's understandable. My intent was to remove pytz only from tests.

Owner

Ch00k replied Nov 21, 2014

Yes, that's understandable. My intent was to remove pytz only from tests.

@joshfriend

This comment has been minimized.

Show comment
Hide comment
@joshfriend

joshfriend Nov 21, 2014

Member

Thanks for doing this, I've hacked together my own ISO8601 serializer/deserializer for a few projects so far which was annoying.

Member

joshfriend commented Nov 21, 2014

Thanks for doing this, I've hacked together my own ISO8601 serializer/deserializer for a few projects so far which was annoying.

@joshfriend joshfriend modified the milestone: 0.2.13 Nov 21, 2014

joshfriend added a commit that referenced this pull request Nov 21, 2014

Merge pull request #345 from Ch00k/iso8601_dt_field
Add support for ISO 8601 formatting to DateTime field

@joshfriend joshfriend merged commit 58e1c9b into flask-restful:master Nov 21, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details

@pyup-bot pyup-bot referenced this pull request Mar 2, 2017

Open

Initial Update #5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment