Convert datetimes to UTC #21

roback · 2016-02-23T11:11:40Z

Query#start_time and #end_time are converted to UTC before making the request.
Post#published and #indexed has their timezone set to UTC.

close #16

Todo

test that use dateutil (to show that that works/is supported) (98bcf5e)
show in examples that you can use pytz or dateutil (cdecac5)

close #16

The methods Query#_ts and Query#_tsTo were almost the same. Also, _tsTo used camelcase which is another reason I wanted to get rid if it :)

Query#start_time and #end_time requires the user of the gem to set timezone, we should set it ourselves too.

Self can not be accessed outside functions so the defaults has to be set in a constructor. This change also fixes the following bug: The attributes declared outside of the constructor, as they were before, resulted in the attributes beeing set on the class and not the instance.

walro · 2016-02-23T13:32:28Z

tests/test_query.py

+        q = self._client.query()
+        q.pattern = "spotify"
+        q.start_time = datetime(2012, 12, 28, 9, 1, 22)
+        with self.assertRaises(twingly_search.TwinglyQueryException):


Maybe you've talked about this already, but if no timezone given I think we should assume UTC rather than raising.

I said we should raise: #16 (comment)

but in the next comment I started to doubt myself... so it's still open for discussion

Ah, thought I had seen it somewhere. Anyway, what does our other libraries do? I was under the impression we don't raise, but I haven't closely followed the latest changes and I can't test now.

To me, at least, it makes sense to assume no tzinfo => UTC.

It's different for Ruby and Node.js, because they default to your local timezone, so we always have that information. We haven't done PHP yet, but PHP will yell at you if you don't configure your timezone or explicitly pick one when creating the date object (as we do in our examples).

Since the standard library has no support for timezones I think we could just assume its UTC. Right now the user is required to add pytz/dateutil (or something similar) to be able to use the client.

Sounds reasonable.

We can have something about it in the README.

Made some changes in 868a5c8

dentarg · 2016-02-23T14:52:58Z

requirements.txt

@@ -1,2 +1,3 @@
 future
 requests
+pytz


we also list requirements in the README, but don't I think we need to do that when we have this file

Yes, I think that requests should be removed from the readme instead :)

Done in 13bef57

dentarg · 2016-02-23T15:52:17Z

What we talked about during fika:

test that use dateutil (to show that that works/is supported)
show in examples that you can use pytz or dateutil

There are two popular date-time related packages in python; pytz and dateutil. Now we have query tests for both of them.

An example explaining how twingly_search handles timezones.

roback · 2016-02-24T11:54:31Z

show in examples that you can use pytz or dateutil

The examples I added only uses dateutil (I went with dateutil because I wanted to use its parse() function).

dentarg · 2016-02-24T15:10:19Z

LGTM :)

Convert datetimes to UTC

* If Query#start_time and #end_time contains timezone information, convert it to UTC (#21) * Parse post tags correctly (#19) * Minor documentation improvements

roback added 5 commits February 23, 2016 10:37

Convert Query#start_time and #end_time to UTC

eefc478

close #16

Remove duplicated code

7179610

The methods Query#_ts and Query#_tsTo were almost the same. Also, _tsTo used camelcase which is another reason I wanted to get rid if it :)

Include timezone in Post#published and #indexed

e6fac64

Query#start_time and #end_time requires the user of the gem to set timezone, we should set it ourselves too.

Set timezone in examples

484add6

walro reviewed Feb 23, 2016
View reviewed changes

roback and others added 2 commits February 23, 2016 15:29

Assume UTC if no timezone is set

868a5c8

Fix typo in readme

145a902

dentarg reviewed Feb 23, 2016
View reviewed changes

roback added 3 commits February 23, 2016 16:55

No need to duplicate requirements

415c6bf

Add tests with time parsed by dateutil

98bcf5e

There are two popular date-time related packages in python; pytz and dateutil. Now we have query tests for both of them.

Add example that uses local timezone

cdecac5

An example explaining how twingly_search handles timezones.

dentarg added 2 commits February 24, 2016 15:39

Merge branch 'master' into utc

2596d04

No need to repeat what's in requirements.txt

13bef57

dentarg added a commit that referenced this pull request Feb 24, 2016

Merge pull request #21 from twingly/utc

4179873

Convert datetimes to UTC

dentarg merged commit 4179873 into master Feb 24, 2016

dentarg deleted the utc branch February 24, 2016 15:21

roback added a commit that referenced this pull request Mar 3, 2016

Release 1.2.0

5d6d8c5

* If Query#start_time and #end_time contains timezone information, convert it to UTC (#21) * Parse post tags correctly (#19) * Minor documentation improvements

Convert datetimes to UTC #21

Convert datetimes to UTC #21

Uh oh!

Conversation

roback commented Feb 23, 2016

Todo

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dentarg commented Feb 23, 2016

Uh oh!

roback commented Feb 24, 2016

Uh oh!

dentarg commented Feb 24, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants