bpo-44653: Support typing types in parameter substitution in the union type.#27247
bpo-44653: Support typing types in parameter substitution in the union type.#27247ambv merged 2 commits intopython:mainfrom
Conversation
serhiy-storchaka
added
type-feature
serhiy-storchaka
force-pushed
the
union-subst-typing
branch
from
bf9d793 to
8f98785
Compare
| if (is_arg_unionable == 0) { | ||
| PyErr_Format(PyExc_TypeError, | ||
| "Each union argument must be a type, got %.100R", arg); | ||
| if (nargs == 0) { |
There was a problem hiding this comment.
I am not sure that this situation is possible now, but just for the case...
| Py_INCREF(res); | ||
| for (Py_ssize_t iarg = 1; iarg < nargs; iarg++) { | ||
| PyObject *arg = PyTuple_GET_ITEM(newargs, iarg); | ||
| Py_SETREF(res, PyNumber_Or(res, arg)); |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Yes, it has quadratic complexity. But I think that most union types has less than 10 args. If it be a problem (many thousands args in real code) we can add some optimizations, but for now I prefer simplicity.
There was a problem hiding this comment.
Good point! Old union de-duplication also had quadratic complexity. So I think it's ok for now. Especially since this is only for substitution, which isn't a common use case either.
Fidget-Spinner
left a comment
Fidget-Spinner
left a comment
There was a problem hiding this comment.
LGTM, just a minor comment about tests.
Lib/test/test_types.py
Outdated
| eq(x[P], int | P | bytes) | ||
| eq(x[typing.Concatenate[int, P]], int | typing.Concatenate[int, P] | bytes) |
There was a problem hiding this comment.
ParamSpec and Concatenate cannot be unioned (or at least it is undefined behavior). So we don't have to test these. They don't throw TypeError because we decided to loosen type checks for them. In the future, these tests might break if their implementation changes. So it's best to leave them out.
There was a problem hiding this comment.
Then it is a separate bug.
There was a problem hiding this comment.
It's not really a bug, we made the decision quite early on to stop advanced runtime type checking new types, and leave it to the type checker or other introspection libraries :). See Guido's comments https://bugs.python.org/msg382858.
There was a problem hiding this comment.
Jest verified -- no, defining __or__ for _TypeVarLike is not a bug (it can make sense for ParamSpec), but in future we need to add additional checks for substitutions.
There was a problem hiding this comment.
Sorry, I wasn't very clear. Yes, __or__ for _TypeVarLike is not a bug during runtime. But using it with union should make no sense to the static type checker. Please see the grammar for the ParamSpec PEP:
https://www.python.org/dev/peps/pep-0612/#valid-use-locations
|
Thanks @serhiy-storchaka for the PR, and @ambv for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10. |
|
GH-27296 is a backport of this pull request to the 3.10 branch. |
…n type. (pythonGH-27247) (cherry picked from commit 2e3744d) Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
6 participants
https://bugs.python.org/issue44653