Add tzinfo= argument to datetime.combine

[abalkin]

BPO	27661
Nosy	@abalkin
Files	issue27661.diff issue27661-2.diff issue27661-3.diff

^{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 2016-08-02.21:49:36.688>
created_at = <Date 2016-07-31.22:12:21.178>
labels = ['type-feature']
title = 'Add tzinfo= argument to datetime.combine'
updated_at = <Date 2016-08-02.21:49:36.686>
user = 'https://github.com/abalkin'

bugs.python.org fields:

activity = <Date 2016-08-02.21:49:36.686>
actor = 'python-dev'
assignee = 'belopolsky'
closed = True
closed_date = <Date 2016-08-02.21:49:36.688>
closer = 'python-dev'
components = []
creation = <Date 2016-07-31.22:12:21.178>
creator = 'belopolsky'
dependencies = []
files = ['43958', '43964', '43965']
hgrepos = []
issue_num = 27661
keywords = ['patch']
message_count = 5.0
messages = ['271751', '271786', '271787', '271788', '271859']
nosy_count = 3.0
nosy_names = ['belopolsky', 'SilentGhost', 'python-dev']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue27661'
versions = ['Python 3.6']

[abalkin]

Add an optional tzinfo argument to datetime.combine() so that

 datetime.combine(d, t, info)

returns the same object as

datetime.combine(d, t).replace(tzinfo=info)

but without creating an intermediate naive instance.

Guido's LGTM: https://mail.python.org/pipermail/datetime-sig/2016-July/000993.html

[abalkin]

The second patch includes documentation changes and addresses review commments.

[abalkin]

From review comments:

Lib/datetime.py:1482: def combine(cls, date, time, tzinfo=True):
On 2016/08/01 08:47:14, SilentGhost wrote:

This strikes me as an odd default value, why not use a more conventional None?

This is the same default as used in the .replace() methods. Arguably, a proper sentinel value would be a better choice, but I think True delivers better performance. In any case, this is not something I would change without consulting with the PyPy folks.

See <http://bugs.python.org/review/27661/diff/18026/Lib/datetime.py#newcode1482\>.

[abalkin]

From review comments:

Lib/datetime.py:1482: def combine(cls, date, time, tzinfo=True):
On 2016/08/01 12:23:12, berkerpeksag wrote:

On 2016/08/01 08:47:14, SilentGhost wrote:
> This strikes me as an odd default value, why not use a more conventional
None?

Agreed. It would also be good to make it keyword-only.

tzinfo is not kw-only in the other constructors and I don't think it should be here. Unlike "fold", tzinfo value is usually recognizable at the call site. It is either called something like "tzinfo", "tz" or "New_York" or is a call such as 'tz.get('US/Eastern').

I would always prefer datetime.combine(d, t, tzinfo) to datetime.combine(d, t, tzinfo=tzinfo). datetime.combine(d, t, New_York) vs. datetime.combine(d, t, tzinfo=New_York) is a closer call, but still the first form is readable enough.

See <http://bugs.python.org/review/27661/diff/18026/Lib/datetime.py#newcode1482\>.

[python-dev]

New changeset adce94a718e3 by Alexander Belopolsky in branch 'default':
Closes bpo-27661: Added tzinfo keyword argument to datetime.combine.
https://hg.python.org/cpython/rev/adce94a718e3