Repository navigation
Use ISO timestamp in diff.py #51831
Description
| BPO | 7582 |
|---|---|
| Nosy | @warsaw, @amauryfa, @abalkin, @djc, @merwok |
| Dependencies | |
| Files |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = 'https://github.com/abalkin'
closed_at = <Date 2012-06-22.17:37:53.631>
created_at = <Date 2009-12-27.15:57:56.577>
labels = ['type-feature']
title = 'Use ISO timestamp in diff.py'
updated_at = <Date 2012-06-22.18:00:30.271>
user = 'https://bugs.python.org/techtonik'bugs.python.org fields:
activity = <Date 2012-06-22.18:00:30.271>
actor = 'techtonik'
assignee = 'belopolsky'
closed = True
closed_date = <Date 2012-06-22.17:37:53.631>
closer = 'belopolsky'
components = ['Demos and Tools']
creation = <Date 2009-12-27.15:57:56.577>
creator = 'techtonik'
dependencies = ['9527', '1647654']
files = ['15678', '15689', '15798', '16184']
hgrepos = []
issue_num = 7582
keywords = ['patch', 'needs review']
message_count = 29.0
messages = ['96912', '96922', '96929', '96962', '96978', '96979', '96981', '97272', '97460', '97471', '97503', '97539', '97540', '97546', '99092', '99096', '99108', '99163', '99166', '99171', '99200', '99393', '104419', '104430', '104439', '107152', '107154', '163434', '163448']
nosy_count = 8.0
nosy_names = ['barry', 'amaury.forgeotdarc', 'belopolsky', 'techtonik', 'djc', 'eric.araujo', 'srumbalski', 'python-dev']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue7582'
versions = ['Python 3.2']
Activity
techtonik commentedon Dec 27, 2009
make diff.py produce unified diffs with ISO 8601 timestamps
Currently generated timestamps are difficult to separate from filename
when parsing if you don't know that the diff was generated by diff.by
This patch make diff.py output more standard compliant by including ISO
8601 timestamps that also conform to RFC 3339 profile for timestamp format
on the internet.
amauryfa commentedon Dec 27, 2009
Using ISO format certainly makes sense, but it seems to me that after
dt=datetime.fromtimestamp(mtime), dt.tzinfo is always None, so the test is
not necessary.
techtonik commentedon Dec 27, 2009
That's true. Is there any way to get current TZ offset in Python? I can't
find anything better than datetime.datetime.now() -
datetime.datetime.utcnow() and then rounding to a nearest minute.
briancurtin commentedon Dec 28, 2009
Look at time.timezone -
http://docs.python.org/library/time.html#time.timezone
techtonik commentedon Dec 28, 2009
Thanks! Now the most elegant patch I could think out.
briancurtin commentedon Dec 29, 2009
Looks cleaner and works for me.
One very minor comment: the spaces inside the parenthesis on the
time.localtime call are inconsistent and don't comply with PEP-8.
techtonik commentedon Dec 29, 2009
Pepeighfied and regenerated. Should be flawless now.
srumbalski commentedon Jan 5, 2010
I think this is incorrect during daylight savings time:
tzoffset = -time.timezone // 60
This should do it:
isdst = time.localtime().tm_isdst
tzoffset = -(time.altzone if isdst else time.timezone)//60
techtonik commentedon Jan 9, 2010
New version detects DST using time.daylight flag.
utcoffset = -(time.altzone if time.daylight else time.timezone) // 60
briancurtin commentedon Jan 9, 2010
Using time.daylight is incorrect. time.daylight specifies the number of hours that the daylight offset is, not a flag to specify whether or not daylight savings time is in effect.
Steven's suggestion of using time.localtime().tm_isdst seems to be the better route.
techtonik commentedon Jan 10, 2010
Brian, documentation says quite the opposite.
time.daylight
Nonzero if a DST timezone is defined.
http://docs.python.org/library/time.html?highlight=daylight#time.daylight
briancurtin commentedon Jan 10, 2010
The documentation could use some work. It means that if the timezone does use a daylight savings time period, time.daylight specifies the amount of the offset. In my timezone this value is 1. However, time.localtime().is_dst is currently 0, because we are not on savings time.
For that reason, using time.daylight in your patch gives an incorrect result for me.
briancurtin commentedon Jan 10, 2010
http://msdn.microsoft.com/en-us/library/2t504ch6%28VS.80%29.aspx has some info for how this is implemented on Windows, and I get the same results on my Mac.
On Linux an AttributeError is raised time.struct_time not having an attribute is_dst, and time.daylight matches the other two platforms.
briancurtin commentedon Jan 10, 2010
Whoops, nevermind the Linux comment on that last one. My typo there caused the exception. The result there is the same as the other platforms.
6 remaining items
techtonik commentedon Feb 10, 2010
I use this tool in instructions how to generate patches on windows, so I am interested to see this fix in the version, that users will likely to use for next couple of years, but I'd be happy to see this committed in any branch.
If it is going to be committed in alpha only them I'd like to propose to change default diff format to unicode as well.
Please, also review http://bugs.python.org/issue7585 that contains more important API change that also prevents diff.py from generating correct patches.
amauryfa commentedon Feb 10, 2010
techtonik commentedon Feb 11, 2010
I am sorry - too much windows to reply. This must be *unified*, of course.
techtonik commentedon Feb 16, 2010
Ok. Let's commit it at least to 2.7 - I'll create a separate issue for discussion of unified diff format later.
techtonik commentedon Apr 28, 2010
So, what about 2.7 ?
briancurtin commentedon Apr 28, 2010
2.7 is now frozen as far as new features go. It's still good for 3.2.
I think this is ready to go, so I'll probably commit it later in the day.
techtonik commentedon Apr 28, 2010
I still do not understand your policy - it is a tool, it is not a part of standard library.
techtonik commentedon Jun 5, 2010
Adding Alexander as ISTM he may be interested to read the time discussion.
abalkin commentedon Jun 5, 2010
The latest patch will produce wrong results if the file was last modified before timezone rules changed in your location. See bpo-1647654.
[-][patch] diff.py to use iso timestamp[/-][+]Use ISO timestamp in diff.py[/+]python-dev commentedon Jun 22, 2012
New changeset ec95b94ea831 by Alexander Belopolsky in branch 'default':
Issue bpo-7582: Use ISO timestamp in diff.py
http://hg.python.org/cpython/rev/ec95b94ea831
techtonik commentedon Jun 22, 2012
Thanks. I am glad OS TZ support finally issue took off. =)