New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Unhelpful error message when one calls a subclass of type with a custom metaclass #71344
Comments
>>>class x(type):pass
>>> x(x)
Traceback (most recent call last):
File "<pyshell#335>", line 1, in <module>
x(x)
TypeError: type() takes 1 or 3 arguments I am giving it one argument, and yet it's complaining that |
Works for me: >>> class X(type): pass
...
>>> X(X)
<class 'type'> |
This issue only happens when the type in question has a custom metaclass: >>> class meta(type):pass
>>> class X(type,metaclass=meta):pass
>>> X(X)
[Same unhelpful TypeError] |
Also happens on 2.7, although you have to declare the metaclass using |
In Objects/typeobject.c#L2290, the code checks that the (meta?)class is exactly Changing the call from Also, there may be performance concerns here. |
Further testing reveals that this issue has nothing to do with metaclasses:
raises the same TypeError. |
Ignore the first part of my previous comment; I improperly tested that. |
Yes, that would be preferable. The error message is at Objects/typeobject.c#l2301, but keep in mind that this message is shown for both faulty calls to type() as well as any of its subclasses that don't override __new__, and I'm lukewarm on adding e.g. a PyType_Check call before that; might as well replace the PyType_CheckExact call and make this work. I'm not knowledgeable enough in that field though, you'll need a core dev's advice. |
I am unable to replicate this in Python 2.7, 3.3 or 3.6. I haven't bothered to test against other versions, because I think that this is a PyShell issue, not a Python issue. (I think you are using PyShell, based on the traceback given.) Before reporting bugs in the interpreter, please test using the vanilla Python interactive interpreter, and not in an enhanced shell or IDE (iPython, PyShell, Spyder, etc). I'm closing the task, please don't re-open unless you can demonstrate the issue in the plain Python interpreter. But please do report it to the PyShell maintainers. |
steven.daprano, you don't appear to have properly read the issue comments. I originally underspecified the conditions necessary to reproduce this, producing Emanuel Barry's closure. I then added a proper reproducer in the third comment, which does work in the vanilla python interpreter. |
When I hit this recently I assumed that that error message meant I'd screwed up the metaclass definition. But if there's a way to improve the error message that would be great. I'll add this to the list of bugs I'm making for the sprints, perhaps one of the sprinters can convince a knowlegable core dev to look at it and render an opinion :) |
On Mon, May 30, 2016 at 01:43:22AM +0000, ppperry wrote:
Ack; I saw your comment about the metaclass, then saw your retraction |
All types are instances of >>> X = type('X', (type,), {})
>>> type(X)
<class 'type'>
>>> isinstance(type(X), type)
True OTOH, implementing this for subclasses of >>> X(X)
<class 'type'>
>>> isinstance(X(X), X)
False PyType_CheckExact(metatype) isn't checking that metatype is >>> Y = X('Y', (X,), {})
>>> Y(Y)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: type() takes 1 or 3 arguments Maybe I'm missing something, but it makes more sense to me if metatype is required to be exactly /* Special case: type(x) should return x->ob_type */
if (metatype == &PyType_Type) {
const Py_ssize_t nargs = PyTuple_GET_SIZE(args);
const Py_ssize_t nkwds = kwds == NULL ? 0 : PyDict_Size(kwds);
if (nargs == 1 && nkwds == 0) {
PyObject *x = PyTuple_GET_ITEM(args, 0);
Py_INCREF(Py_TYPE(x));
return (PyObject *) Py_TYPE(x);
}
/* SF bug 475327 -- if that didn't trigger, we need 3
arguments. but PyArg_ParseTupleAndKeywords below may give
a msg saying type() needs exactly 3. */
if (nargs + nkwds != 3) {
PyErr_SetString(PyExc_TypeError,
"type() takes 1 or 3 arguments");
return NULL;
}
} This change yields the following behavior:
>>> type(X)
<class 'type'>
>>> type(1, 2)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: type() takes 1 or 3 arguments
>>> X()
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: Required argument 'name' (pos 1) not found
>>> X(X)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: type() argument 1 must be str, not type |
+1. I prefer that change, as using subclasses of Attached patch implements Eryk's suggestion. I haven't found any tests that checked for subclasses of type specifically (except tests testing for metaclass stuff), and I haven't added any either. |
Guido, this is your code. Do you care to opine? |
I don't recall writing that any more, but that fix looks right. (Though why write |
If only https://hg.python.org/cpython/file/v2.2a3/Objects/typeobject.c#l631 I don't know why the final release of 2.2 switched to using PyType_CheckExact, which is true for most metaclasses. That's why I feel like I'm missing something here. Probably it used PyType_CheckExact instead of PyType_Check to ensure PyType_IsSubtype wouldn't be called. Nowadays that's optimized away via PyType_FastSubclass and the Py_TPFLAGS_TYPE_SUBCLASS flag (set up in inherit_special). If it's decided to retain this special case for metaclasses other than |
Yes, Before the patch: >>> type(1, 2, 3)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: type() argument 1 must be str, not int After the patch: >>> type(1, 2, 3)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: type.__new__() argument 1 must be str, not int
>>> class X(type): pass
...
>>> X(1)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: type.__new__() argument 1 must be str, not int |
OK, the patch looks fine. Please treat this as a new feature (just in case) and only apply it to 3.6. |
Fair enough. Should this get a note in What's new? Possibly in the "Changes in Python API" section. |
No, just an entry in Misc/NEWS. --Guido (mobile) |
Needed tests and updating the documentation. |
New patch with tests and documentation. I didn't really know where to put the tests; test_types seemed reasonable to me (other option was test_metaclass). |
Please don't mark your own patch as 'commit review'. |
Berker, I don't mind if people mark their own patches for commit review *when they think it is ready for commit*. (Just as with reviewers, if they start being consistently right, they are ready for commit privs :) However, a non-committer setting a patch to commit review is waving a hand for attention from a core dev, and that should only be done *after* a patch has been *reviewed* and judged ready for final review before commit. (I broke my own rule the other day and got in trouble, though...I set a patch to commit review to remind myself to review it rather than because I thought it was commit ready already, and it got committed...) |
Thanks for the patch, Emanuel. I left some comments about Sphinx markup on Rietveld.
Is there any person who really thinks that their own patch is *not* ready for commit review? :) |
New patch with Berker's comments. I'm really not used to Sphinx markup so thanks for that!
Partial patches aren't that uncommon in some projects. I also sometimes don't trust that I got everything right (especially if it's in a very large codebase like CPython), and having a few more pair of eyes taking a look at it helps. |
Ping |
Rebased patch so that it applies cleanly again. |
New changeset c3498187f998 by Berker Peksag in branch 'default': |
Thanks! |
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:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: