logging.Formatter attributes fixed

[ambv]

Fixes #720.

Related changes: used a NamedTuple for time.struct_time on Python 3, used
sys.version selectors for proper typing of #716, added missing *Style classes
to logging on Python 3.

[ambv]

(the NamedTuple was already used on Python 2)

[gvanrossum]

I don't think this will work right. I tried this simple example:

T = Union[Tuple[int, int], Tuple[int, int, str]]
def f() -> T: pass
a = f()
reveal_type(a[0])
reveal_type(a[2])

and mypy revealed that both a[0] and a[2] have type Any. Without the union it understands that they are int and str, respectively.

[ambv]

I see what you're saying. Two thoughts.

1. This looks like a possible future improvement to Mypy? I don't see the current behavior specified in PEP 484.
2. The main point of the TimeTuple type is checking arguments to functions (no such name in the actual time module). In this context, it works exactly as expected. See example below.

import time
print(time.struct_time((1,2)))

At runtime this fails with:

Traceback (most recent call last):
  File "/tmp/qwe.py", line 10, in <module>
    print(time.struct_time((1,2)))
TypeError: time.struct_time() takes an at least 9-sequence (2-sequence given)

With my change to time.pyi, Mypy correctly fails as well:

/tmp/qwe.py:10: error: Argument 1 to "struct_time" has incompatible type "Tuple[int, int]"; expected "Union[Tuple[int, int, int, int, int, int, int, int, int], Tuple[int, int, int, int, int, int, int, int, int, str], Tuple[int, int, int, int, int, int, int, int, int, str, int]]"

So, I'd file an issue against the current behavior of Mypy and accept the current union, esp. that time.pyi has no API that returns TimeTuples, it's just a function argument. It also works for type mismatches for tuple fields:

/tmp/qwe.py:2: error: Argument 1 to "struct_time" has incompatible type "Tuple[str, str, str, str, str, str, str, str, str]"; expected "Union[Tuple[int, int, int, int, int, int, int, int, int], Tuple[int, int, int, int, int, int, int, int, int, str], Tuple[int, int, int, int, int, int, int, int, int, str, int]]"

[gvanrossum]

OK, filed python/mypy#2509. PEP 484 in general is way under-specified.

I'm still hesitant to accept this, although a bit less so given that there are no APIs returning TimeTuple. (However, there's another definition of TimeTuple in datetime, and that is returned by some methods. However, those stubs should be changed to return struct_time.)

[ambv]

Regardless of the current implementation state of the type checker, what do you consider a valid type definition for TimeTuple? The old one was overly restrictive.

[gvanrossum]

In what sense is it overly restrictive? All functions that take a TimeTuple also take a struct_time; IMO you should always use the latter when specifying the extra values. The struct_time, when unpacked, always returns exactly 9 values (even if it stores extra values).

The docs are pretty vague; the best I can find is here: https://docs.python.org/3/library/time.html?highlight=mktime#time.mktime -- this still says 9-tuple. Also note the definition of https://docs.python.org/3/library/time.html?highlight=mktime#time.struct_time which says that tm_gmtoff and tm_zone are only available on platforms where the C library supports these.

[ambv]

Oh my, you are absolutely correct. time.mktime and time.strftime only accept 9-tuples or struct_times! Sorry for the back and forth, restored TimeTuple as a 9-tuple.

[ambv]

(restored TimeTuple to a 9-tuple)

[gvanrossum]

Sorry, another union-related qualm... Then I'll stop!

[gvanrossum]

Is it even worth keeping the union here? Unions are a pain to use.

[ambv]

You're right, the only "int" ever returned is 0 which for all intents and purposes is equivalent to False unless somebody does "is False" checks. Will simplify further.

[ambv]

Fixed.

[gvanrossum]

Thanks! (Next time when responding to a code review can you not squash/amend local commits? We squash on merge.)