[issue20473] inspect.Signature no longer handles builtin classes correctly
Yury Selivanov added the comment: And also right now, inspect.signature looks for '__text_signature__' when no used-defined __init__ was found. That's also going to be changed, but again, when __text_signature__ becomes a public documented API. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20473 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20473] inspect.Signature no longer handles builtin classes correctly
Larry Hastings added the comment: I don't think __text_signature__ should ever be a documented public API. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20473 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20473] inspect.Signature no longer handles builtin classes correctly
Yury Selivanov added the comment: FWIW, I think the same. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20473 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20473] inspect.Signature no longer handles builtin classes correctly
Yury Selivanov added the comment: And also, reading Stefan in another issue, I'm a bit worried that it may forcibly become a public API. Users tend to start using APIs before they are public, and that's especially true for python dunder attributes. Maybe we should document '__text_signature__' and 'sig=', and explicitly state that it's a part of CPython private API, that likely to have some semantics/syntax changed in 3.5? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20473 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20473] inspect.Signature no longer handles builtin classes correctly
Larry Hastings added the comment: Also also, I remember specifically that the isinstance(type) code would fail builtin classes. Could you please find an example of this? This was during the development of the original feature. I changed the if statement for the from_builtin() call so it'd accept type objects too. But it never got a chance to see any, because the check for type objects above it would see that it was a type, see that it was a builtin type, and raise an exception. That's why I moved the if statement with the from_builtin() call to the top of the function, so it would get the first chance to examine the callable. This was just historical context, and I'm sure you already solved the problem in an equivalent way. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20473 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20473] inspect.Signature no longer handles builtin classes correctly
Yury Selivanov added the comment: Larry, Quoting your reply from #17159: Also also, I remember specifically that the isinstance(type) code would fail builtin classes. Could you please find an example of this? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20473 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20473] inspect.Signature no longer handles builtin classes correctly
Roundup Robot added the comment: New changeset c19f5e4fdbe0 by Yury Selivanov in branch 'default': inspect.signature: Add (restore) support for builtin classes #20473 http://hg.python.org/cpython/rev/c19f5e4fdbe0 -- nosy: +python-dev ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20473 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20473] inspect.Signature no longer handles builtin classes correctly
Yury Selivanov added the comment: Committed. We'll likely need to modify the code again in 3.5, once we settle the exact semantics of __text_signature__. -- resolution: - fixed status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20473 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20473] inspect.Signature no longer handles builtin classes correctly
Larry Hastings added the comment: What semantics are unsettled? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20473 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20473] inspect.Signature no longer handles builtin classes correctly
Yury Selivanov added the comment: For instance if __text_signature__ and __signature__ are present simultaneously which one should be used, or the use of '($' to specify bound-methods first parameters etc. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20473 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20473] inspect.Signature no longer handles builtin classes correctly
Yury Selivanov added the comment: OK, I'll take a look tomorrow. Don't think it's related to #20471 though, as in there, if fails to find a signature for the 'builtin.object' (but I may be wrong). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20473 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20473] inspect.Signature no longer handles builtin classes correctly
Yury Selivanov added the comment: OK, there was no unit-test for this... looking into the problem, the first questing is: why is __text_signature__ is on the '_pickle.Pickler' object, not on '_pickle.Pickler.__init__'? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20473 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20473] inspect.Signature no longer handles builtin classes correctly
Larry Hastings added the comment: Because slots like tp_init and tp_call don't have docstrings. So it has to go in the class's docstring. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20473 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20473] inspect.Signature no longer handles builtin classes correctly
Larry Hastings added the comment: I meant to say slots like tp_new and tp_init. But fwiw it's true of tp_call too. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20473 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20473] inspect.Signature no longer handles builtin classes correctly
Larry Hastings added the comment: And, I don't see how your changes to inspect.py could have caused the failures on the buildbot either. But, then, I don't see how *anything* could cause the failures on the buildbot. And your changes to inspect.py happened at (I think) roughly the same time. Correlation isn't causation, but it's all I have to go on right now. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20473 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20473] inspect.Signature no longer handles builtin classes correctly
Larry Hastings added the comment: Stefan Krah suggests that the failure in 20473 is because that platform builds without docstrings, and the test requires them. So that should be an easy fix. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20473 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20473] inspect.Signature no longer handles builtin classes correctly
Yury Selivanov added the comment: Stefan Krah suggests that the failure in 20473 is because that platform builds without docstrings, and the test requires them. So that should be an easy fix. Good news ;) OK, I'll decorate the test. Because slots like tp_init and tp_call don't have docstrings. So it has to go in the class's docstring. So what's going on and why is it broken: - from_builtin wasn't checking the incoming 'func' argument's type, it started to work with its '__text_signature__' right away. That was fixed. - in 'inspect.signature' we checked (isinstance(obj, _NonUserDefinedCallables) or ismethoddescriptor(obj) or isinstance(obj, type)) so *classes* were tried in 'from_builtin' too. And that's why it worked. Any class (even used-defined in pure Python) that had '__text_signature__' was passed to the 'Signature.from_builtin' and it did the right job. Now, I don't want to completely rollback my commits, as I still think that 'from_builtin' should strictly check what object is it working with, and raise appropriate exceptions. What we need to do is to separate parsing of '__text_signature__' into a private inspect module helper, so that 'from_builtin' becomes a tiny wrapper. Then, in 'signature.inspect' we need to find a way of testing if the given object is a builtin class, and call the parsing helper on it if it has the '__text_signature__' attribute. I'm looking into this. BTW, are you sure we can't somehow add '__text_signature__' to builtin class' '__init__'? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20473 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20473] inspect.Signature no longer handles builtin classes correctly
Changes by Yury Selivanov yselivanov...@gmail.com: -- nosy: +ncoghlan ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20473 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20473] inspect.Signature no longer handles builtin classes correctly
Nick Coghlan added the comment: Those are C level descriptors, so we'd have to add new fields to the structs, and that's not going to happen at this stage of the release cycle. However, there's also the fact that tp_new and tp_init are required to have the *same* signature, and those override the apparent signature of __call__ on the metaclass, so it's actually better to just document it once on the class. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20473 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20473] inspect.Signature no longer handles builtin classes correctly
Yury Selivanov added the comment: Please review the attached patch (sig_builtins_01.patch). Some details: - All parsing code from Signature.from_builtin was moved in a separate helper '_signature_fromstr' - Signature.from_builtin calls '_signature_fromstr'. All its validation logic is untouched. - 'inspect.signature' was tweaked a bit: when it's certain that the object is a class and there is no user-defined __init__ or __new__ or its meta's __call__, it traverses the MRO to find non-empty __text_signature__. If it finds one -- it returns with _signature_fromstr(). If not, it checks if __init__ is type.__init__ or object.__init__. The last check is a bit tricky -- the only way of doing that check (I think) is to use __qualname__. Since the patch is non-trivial, any review/comments would be greatly appreciated. -- keywords: +patch Added file: http://bugs.python.org/file33853/sig_builtincls_01.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20473 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20473] inspect.Signature no longer handles builtin classes correctly
Changes by Yury Selivanov yselivanov...@gmail.com: Added file: http://bugs.python.org/file33858/sig_builtincls_02.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20473 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20473] inspect.Signature no longer handles builtin classes correctly
Yury Selivanov added the comment: Second patch attached: sig_builtincls_02.patch, with a fix, that Nick suggested. Larry, I'd like you to take a quick look at it as well, before I commit it. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20473 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20473] inspect.Signature no longer handles builtin classes correctly
New submission from Larry Hastings: Yury: In revision 9433b380ad33 you changed inspect.Signature so that it cannot handle builtin classes. Please fix it. import _pickle import inspect str(inspect.signature(_pickle.Pickler)) '()' _pickle.Pickler.__text_signature__ '(file, protocol=None, fix_imports=True)' Those two strings should be the same. I don't know any guaranteed way to tell a builtin class from a user class. So if you pass in a class, the best approach is to do what it used to do: try from_builtin, and if it fails fail over to the isinstance(obj, type) code. You changed it to if _signature_is_builtin(obj): return Signature.from_builtin(obj) This unambiguously returns the result from from_builtin. Either find a way that you can determine a class is a builtin 100% reliably, or change this to *try* from_builtin but only return its result if it's successful. Your changes might have also caused #20471; that wasn't failing before. I'm still investigating. -- assignee: yselivanov messages: 209871 nosy: larry, yselivanov priority: normal severity: normal stage: needs patch status: open title: inspect.Signature no longer handles builtin classes correctly type: behavior versions: Python 3.4 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20473 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
