| msg327558 - (view) |
Author: Alexander Böhn (fish2000) * |
Date: 2018-10-12 03:03 |
| When creating a new type with `typing.NewType(…)` the result is a function defined on-the-fly with no reasonable `__repr__` definition – the string representation of the type looks something like e.g. “<function NewType..new_type at 0x108ae6950>” which isn’t very useful or aesthetically appealing. |
|
|
| msg327595 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2018-10-12 16:38 |
| For repr you need to set __qualname__, not just __name__. It may be worth to set also __module__ for pickleability and better help. |
|
|
| msg327596 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2018-10-12 16:42 |
| You can determine the module of the caller by using sys._getframe(). |
|
|
| msg327610 - (view) |
Author: Alexander Böhn (fish2000) * |
Date: 2018-10-12 20:10 |
| The proposed solution in the PR replaces the identity-function return value of `NewType(…)` with a callable class instance that adds an explicit `__repr__` function – which that function cobbles together the string representations of the supplied type and the new types’ name into a sensible and stylistically consistent “repr” output. In this capacity the `__name__` vs. `__qualname__` consideration would appear to be an implementation detail – in this case, calculating a `__qualname__` value, á la PEP-3155, doesn’t seem to be possible nor advantageous, in this specific case. On the other hand, adding `__module__` seems like it’d be a win – although I am wary about introducing a dependency on a function that starts with an underscore. How reliable and/or portable is `sys._getframe()` ? |
|
|
| msg327611 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2018-10-12 20:20 |
| There are disadvantages of using a callable class. First, creating a class is 10-100 times slower than creating a function. Second, calling it is slower than calling a function. sys._getframe() is already used in the typing module (and also in collections and enum). |
|
|
| msg327968 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2018-10-18 11:13 |
| PR 9951 is a simpler way. It just sets __qualname__ (and __module__ for completeness). >>> import typing >>> UserId = typing.NewType('UserId', int) >>> UserId <function UserId at 0x7f1386ac77e0> |
|
|
| msg328224 - (view) |
Author: Ivan Levkivskyi (levkivskyi) * |
Date: 2018-10-21 22:06 |
| OK, so now we have two "competing" PRs. I think I like Serhiy's PR a bit more, since it is simpler. I would propose to look at benchmarks and then decide. Are there any other factors I am missing for choosing one or other approach? |
|
|
| msg328699 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2018-10-28 16:13 |
| PR 9951 is simpler, but it makes invoking NewType() slower, because retrieving the module from the caller frame is slow in comparison with other operations. $ ./python -m timeit -s "import typing" -- "UserId = typing.NewType('UserId', int)" PR 9808: 1000000 loops, best of 5: 316 nsec per loop PR 9951: 500000 loops, best of 5: 634 nsec per loop PR 9951 without setting __module__ and stripping components before dot in __name__: 1000000 loops, best of 5: 316 nsec per loop In contrary, calling the resulting type is slower with PR 9808: $ ./python -m timeit -s "import typing" -s "UserId = typing.NewType('UserId', int)" -- "UserId(5)" PR 9808: 2000000 loops, best of 5: 105 nsec per loop PR 9951: 5000000 loops, best of 5: 74.1 nsec per loop What other operations should be supported? Should the NewType() result be pickleable? Currently it is not, in contrary to other typing types. If it should be pickleable, should it be pickled by name (resulting in restoring the same instance on unpickling, but failing if it is not accessible by name) or by its arguments (resulting in creating a new instance when unpickled)? Should it support comparison and what values should be treated as equal? If make NewType a class, I would make the repr looking like a call: >>> import typing >>> UserId = typing.NewType('UserId', int) >>> UserId NewType('UserId', <class 'int'>) >>> UserNames = typing.NewType('UserNames', typing.List[int]) >>> UserNames NewType('UserNames', typing.List[int]) |
|
|
| msg328707 - (view) |
Author: Ivan Levkivskyi (levkivskyi) * |
Date: 2018-10-28 16:51 |
| Serhiy, thanks for benchmarks and good suggestions! > If make NewType a class, I would make the repr looking like a call This is a nice idea, it indeed looks better. We can probably also use `_type_repr()` helper for the argument (for consistency). > Should the NewType() result be pickleable? This is not required by PEP 484, but I could imagine this may be useful. > If it should be pickleable, should it be pickled by name (resulting in restoring the same instance on unpickling, but failing if it is not accessible by name) or by its arguments (resulting in creating a new instance when unpickled)? Logically, they should behave like class objects, so if we are going to make them pickleable, they should be pickled by full name. > Should it support comparison and what values should be treated as equal? Again, since they should behave like class objects, I think we don't need any dedicated `__eq__`, they should be just compared by identity. From performance point of view, both PRs look good. Maybe we can even combine both, so that we have both fine-tuned repr and pickleability. I understand this means we will get slower benchmarks for both creation and instantiation, but I think it is still OK, since it is still much faster than creating a new class. |
|
|
| msg343333 - (view) |
Author: Cheryl Sabella (cheryl.sabella) * |
Date: 2019-05-23 22:54 |
| Serhiy, It looks like Ivan approved your PR and was ready to merge it, but left it up to you for the final decision about incorporating changes from the competing PR. This is just a friendly ping in case you want to get it in for 3.8. Thanks! |
|
|
| msg398065 - (view) |
Author: Yurii Karabas (uriyyo) *  |
Date: 2021-07-23 15:20 |
| `NewType` was converted into callable class at scope of https://bugs.python.org/issue44353 Currently `repr` of `NewType` is: ``` >>> UserId = NewType("UserId", int) __main__.UserId ``` |
|
|