solderpunk
/
gemfeed
2
7
Fork 5
Code Issues 1 Pull Requests 3 Releases Wiki Activity

Use local timezone for filename dates #3

Open
makeworld wants to merge 1 commits from makeworld/gemfeed:master into master
pull from: makeworld/gemfeed:master
merge into: solderpunk:master
Conversation 5 Commits 1 Files Changed 2 +3 -2
makeworld commented 2020-12-05 00:09:22 +00:00
First-time contributor
Copy Link

Fixes #1

Source for converting to local timezone: https://stackoverflow.com/a/13287083/7361270

This solution only supports Python 3.3+. Please let me know if that's a problem.

Also, I'd appreciate if a new version of gemfeed could be released if this gets merged.

Fixes #1 Source for converting to local timezone: https://stackoverflow.com/a/13287083/7361270 This solution only supports Python 3.3+. Please let me know if that's a problem. Also, I'd appreciate if a new version of gemfeed could be released if this gets merged.
makeworld added 1 commit 2020-12-05 00:09:26 +00:00
solderpunk commented 2020-12-06 14:55:12 +00:00
Owner
Copy Link

Thanks for this. I was just looking at the datetime docs to make sure I understood and agreed with what this code did, and now I'm a little confused. I think the call to replace is redundant, but the bigger problem is that astimezone doesn't seem to work like it should for me?

>>> import datetime
>>> datetime.datetime.utcnow()
datetime.datetime(2020, 12, 6, 14, 51, 26, 811369)
>>> datetime.datetime.utcnow().astimezone()
datetime.datetime(2020, 12, 6, 14, 51, 27, 977788, tzinfo=datetime.timezone(datetime.timedelta(seconds=3600), 'CET'))

That's weird, right? It's clearly aware that I'm in CET and not UTC, and adds the tzinfo accordingly, but it doesn't adjust the hour value accordingly, even though it is supposed to take care of "adjusting the date and time data so the result is the same UTC time as self, but in tz’s local time". If it worked properly, there should be no difference between the following - the first is just the current time in UTC, and the latter converts from that to local time and then back to UTC - but I get different times:

>>> datetime.datetime.utcnow()
datetime.datetime(2020, 12, 6, 14, 54, 20, 425199)
>>> datetime.datetime.utcnow().astimezone().astimezone(tz=datetime.timezone.utc)
datetime.datetime(2020, 12, 6, 13, 54, 20, 694474, tzinfo=datetime.timezone.utc)

Is this working properly on your end?

Thanks for this. I was just looking at the `datetime` docs to make sure I understood and agreed with what this code did, and now I'm a little confused. I think the call to `replace` is redundant, but the bigger problem is that `astimezone` doesn't seem to work like it should for me? ``` >>> import datetime >>> datetime.datetime.utcnow() datetime.datetime(2020, 12, 6, 14, 51, 26, 811369) >>> datetime.datetime.utcnow().astimezone() datetime.datetime(2020, 12, 6, 14, 51, 27, 977788, tzinfo=datetime.timezone(datetime.timedelta(seconds=3600), 'CET')) ``` That's weird, right? It's clearly aware that I'm in CET and not UTC, and adds the `tzinfo` accordingly, but it doesn't adjust the hour value accordingly, even though it is supposed to take care of "adjusting the date and time data so the result is the same UTC time as self, but in tz’s local time". If it worked properly, there should be no difference between the following - the first is just the current time in UTC, and the latter converts from that to local time and then back to UTC - but I get different times: ``` >>> datetime.datetime.utcnow() datetime.datetime(2020, 12, 6, 14, 54, 20, 425199) >>> datetime.datetime.utcnow().astimezone().astimezone(tz=datetime.timezone.utc) datetime.datetime(2020, 12, 6, 13, 54, 20, 694474, tzinfo=datetime.timezone.utc) ``` Is this working properly on your end?
makeworld commented 2020-12-06 15:10:20 +00:00
Author
First-time contributor
Copy Link

After some experimentation of my own, I've determined that the replace is not redundant particularly for situations like the one you've presented. It turns out that datetime.utcnow() is a bad function to use in general, because it doesn't set the timezone to UTC, it just leaves it as None:

>>> datetime.datetime.utcnow()
datetime.datetime(2020, 12, 6, 15, 0, 19, 869653)
>>> datetime.datetime.utcnow().tzinfo
>>> print(datetime.datetime.utcnow().tzinfo)
None
>>> print(datetime.datetime.utcnow().tzinfo)
None
>>> type(datetime.datetime.utcnow().tzinfo)
<class 'NoneType'>

As the documentation for this function notes:

Warning: Because naive datetime objects are treated by many datetime methods as local times, it is preferred to use aware datetimes to represent times in UTC. As such, the recommended way to create an object representing the current time in UTC is by calling datetime.now(timezone.utc).

Using datetime.now(timezone.utc) as the docs recommend works as expected:

>>> datetime.datetime.now(datetime.timezone.utc)
datetime.datetime(2020, 12, 6, 15, 1, 21, 627237, tzinfo=datetime.timezone.utc)
>>> datetime.datetime.now(datetime.timezone.utc).astimezone()
datetime.datetime(2020, 12, 6, 10, 5, 1, 421923, tzinfo=datetime.timezone(datetime.timedelta(days=-1, seconds=68400), 'EST'))

And when you use .replace(tzinfo=timezone.utc) as my code does, even using datetime.utcnow() works fine, because the timezone is set to UTC explicitly first.

>>> datetime.datetime.utcnow()
datetime.datetime(2020, 12, 6, 15, 7, 30, 752058)
>>> datetime.datetime.utcnow().replace(tzinfo=datetime.timezone.utc).astimezone()
datetime.datetime(2020, 12, 6, 10, 7, 44, 461259, tzinfo=datetime.timezone(datetime.timedelta(days=-1, seconds=68400), 'EST'))

This was all in Python 3.8.6, but should apply to Python 3.3+ in general.

You can also test the code overall by creating a dummy file named 2020-12-06-test.gmi or something, and when you look at the feed gemfeed creates, you'll see the timestamps have your local timezone in them.

After some experimentation of my own, I've determined that the `replace` is not redundant particularly for situations like the one you've presented. It turns out that `datetime.utcnow()` is a bad function to use in general, because it doesn't set the timezone to UTC, it just leaves it as `None`: ```python >>> datetime.datetime.utcnow() datetime.datetime(2020, 12, 6, 15, 0, 19, 869653) >>> datetime.datetime.utcnow().tzinfo >>> print(datetime.datetime.utcnow().tzinfo) None >>> print(datetime.datetime.utcnow().tzinfo) None >>> type(datetime.datetime.utcnow().tzinfo) <class 'NoneType'> ``` As the documentation for this function notes: > **Warning:** Because naive `datetime` objects are treated by many `datetime` methods as local times, it is preferred to use aware datetimes to represent times in UTC. As such, the recommended way to create an object representing the current time in UTC is by calling `datetime.now(timezone.utc)`. Using `datetime.now(timezone.utc)` as the docs recommend works as expected: ```python >>> datetime.datetime.now(datetime.timezone.utc) datetime.datetime(2020, 12, 6, 15, 1, 21, 627237, tzinfo=datetime.timezone.utc) >>> datetime.datetime.now(datetime.timezone.utc).astimezone() datetime.datetime(2020, 12, 6, 10, 5, 1, 421923, tzinfo=datetime.timezone(datetime.timedelta(days=-1, seconds=68400), 'EST')) ``` And when you use `.replace(tzinfo=timezone.utc)` as my code does, even using `datetime.utcnow()` works fine, because the timezone is set to UTC explicitly first. ```python >>> datetime.datetime.utcnow() datetime.datetime(2020, 12, 6, 15, 7, 30, 752058) >>> datetime.datetime.utcnow().replace(tzinfo=datetime.timezone.utc).astimezone() datetime.datetime(2020, 12, 6, 10, 7, 44, 461259, tzinfo=datetime.timezone(datetime.timedelta(days=-1, seconds=68400), 'EST')) ``` This was all in Python 3.8.6, but should apply to Python 3.3+ in general. --- You can also test the code overall by creating a dummy file named `2020-12-06-test.gmi` or something, and when you look at the feed gemfeed creates, you'll see the timestamps have your local timezone in them.
solderpunk commented 2020-12-06 16:37:29 +00:00
Owner
Copy Link

Ah, thanks a lot for clarifying! Shame on utcnow! However, the use of replace in your commit is still redundant, I think, because the initial datetime object is created by calling strptime on a string which ends in "Z", so it gets a proper UTC timezone assigned. Am I right in thinking that if we removed the "Z" suffix from date then return datetime.datetime.strptime(date, "%Y-%m-%d").astimezone() would be sufficient for the desired behaviour?

No criticism intended in any of this, by the way! I appreciate you helping out with this, I'm just trying to keep things short and clear, and chaining three different methods together in a single long line seems less than ideal.

Ah, thanks a lot for clarifying! Shame on `utcnow`! However, the use of `replace` in your commit is still redundant, I think, because the initial `datetime` object is created by calling `strptime` on a string which ends in "Z", so it gets a proper UTC timezone assigned. Am I right in thinking that if we removed the "Z" suffix from `date` then ` return datetime.datetime.strptime(date, "%Y-%m-%d").astimezone()` would be sufficient for the desired behaviour? No criticism intended in any of this, by the way! I appreciate you helping out with this, I'm just trying to keep things short and clear, and chaining three different methods together in a single long line seems less than ideal.
makeworld commented 2020-12-06 17:45:51 +00:00
Author
First-time contributor
Copy Link

However, the use of replace in your commit is still redundant, I think, because the initial datetime object is created by calling strptime on a string which ends in "Z", so it gets a proper UTC timezone assigned.

That's correct.

>>> datetime.datetime.strptime("2020-01-01 Z", "%Y-%m-%d %z")
datetime.datetime(2020, 1, 1, 0, 0, tzinfo=datetime.timezone.utc)
>>> datetime.datetime.strptime("2020-01-01 Z", "%Y-%m-%d %z").replace(tzinfo=datetime.timezone.utc).astimezone(tz=None)
datetime.datetime(2019, 12, 31, 19, 0, tzinfo=datetime.timezone(datetime.timedelta(days=-1, seconds=68400), 'EST'))
>>> datetime.datetime.strptime("2020-01-01 Z", "%Y-%m-%d %z").astimezone(tz=None)
datetime.datetime(2019, 12, 31, 19, 0, tzinfo=datetime.timezone(datetime.timedelta(days=-1, seconds=68400), 'EST'))

Am I right in thinking that if we removed the "Z" suffix from date then return datetime.datetime.strptime(date, "%Y-%m-%d").astimezone() would be sufficient for the desired behaviour?

No, I'm not sure if you miswrote something. As you mentioned earlier, by calling strptime on a string that ends in "Z", a proper UTC timezone is assigned. If you don't have the timezone, as you mention here, then timezone would would default to None, and astimezone() will change the zone but not actually adjust the datetime values.

>>> datetime.datetime.strptime("2020-01-01", "%Y-%m-%d")
datetime.datetime(2020, 1, 1, 0, 0)
>>> datetime.datetime.strptime("2020-01-01", "%Y-%m-%d").astimezone()
datetime.datetime(2020, 1, 1, 0, 0, tzinfo=datetime.timezone(datetime.timedelta(days=-1, seconds=68400), 'EST'))
>>> datetime.datetime.strptime("2020-01-01", "%Y-%m-%d").replace(tzinfo=datetime.timezone.utc).astimezone()
datetime.datetime(2019, 12, 31, 19, 0, tzinfo=datetime.timezone(datetime.timedelta(days=-1, seconds=68400), 'EST'))

This basically leaves two options for changing the code. Either use the "Z" and theline becomes datetime.datetime.strptime(date, "%Y-%m-%d %z").astimezone(tz=None), or leave off the "Z" and the line is datetime.datetime.strptime(date, "%Y-%m-%d").replace(tzinfo=datetime.timezone.utc).astimezone(tz=None).

Which one do you prefer?

> However, the use of `replace` in your commit is still redundant, I think, because the initial `datetime` object is created by calling `strptime` on a string which ends in "Z", so it gets a proper UTC timezone assigned. That's correct. ```python >>> datetime.datetime.strptime("2020-01-01 Z", "%Y-%m-%d %z") datetime.datetime(2020, 1, 1, 0, 0, tzinfo=datetime.timezone.utc) >>> datetime.datetime.strptime("2020-01-01 Z", "%Y-%m-%d %z").replace(tzinfo=datetime.timezone.utc).astimezone(tz=None) datetime.datetime(2019, 12, 31, 19, 0, tzinfo=datetime.timezone(datetime.timedelta(days=-1, seconds=68400), 'EST')) >>> datetime.datetime.strptime("2020-01-01 Z", "%Y-%m-%d %z").astimezone(tz=None) datetime.datetime(2019, 12, 31, 19, 0, tzinfo=datetime.timezone(datetime.timedelta(days=-1, seconds=68400), 'EST')) ``` > Am I right in thinking that if we removed the "Z" suffix from `date` then ` return datetime.datetime.strptime(date, "%Y-%m-%d").astimezone()` would be sufficient for the desired behaviour? No, I'm not sure if you miswrote something. As you mentioned earlier, by calling `strptime` on a string that ends in "Z", a proper UTC timezone is assigned. If you don't have the timezone, as you mention here, then timezone would would default to `None`, and `astimezone()` will change the zone but not actually adjust the datetime values. ```python >>> datetime.datetime.strptime("2020-01-01", "%Y-%m-%d") datetime.datetime(2020, 1, 1, 0, 0) >>> datetime.datetime.strptime("2020-01-01", "%Y-%m-%d").astimezone() datetime.datetime(2020, 1, 1, 0, 0, tzinfo=datetime.timezone(datetime.timedelta(days=-1, seconds=68400), 'EST')) >>> datetime.datetime.strptime("2020-01-01", "%Y-%m-%d").replace(tzinfo=datetime.timezone.utc).astimezone() datetime.datetime(2019, 12, 31, 19, 0, tzinfo=datetime.timezone(datetime.timedelta(days=-1, seconds=68400), 'EST')) ``` This basically leaves two options for changing the code. Either use the "Z" and theline becomes `datetime.datetime.strptime(date, "%Y-%m-%d %z").astimezone(tz=None)`, or leave off the "Z" and the line is `datetime.datetime.strptime(date, "%Y-%m-%d").replace(tzinfo=datetime.timezone.utc).astimezone(tz=None)`. Which one do you prefer?
makeworld commented 2020-12-31 00:30:45 +00:00
Author
First-time contributor
Copy Link

@solderpunk bump :)

I believe I've answered your most recent comment, and now I have a question about which version of the line you like best, as you can see above. Once you let me know I'll update the file, and this PR will be ready to merge. Of course, it works fine the way it is now as well.

@solderpunk bump :) I believe I've answered your most recent comment, and now I have a question about which version of the line you like best, as you can see above. Once you let me know I'll update the file, and this PR will be ready to merge. Of course, it works fine the way it is now as well.
This pull request can be merged automatically.
You are not authorized to merge this pull request.
You can also view command line instructions.

Step 1:

From your project repository, check out a new branch and test the changes.
git checkout -b makeworld-master master
git pull master

Step 2:

Merge the changes and update on Gitea.
git checkout master
git merge --no-ff makeworld-master
git push origin master
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: solderpunk/gemfeed#3
No description provided.
Delete Branch "makeworld/gemfeed:master"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?