Bugs item #1683368, was opened at 2007-03-18 21:32
Message generated for change (Comment added) made by rhamphoryncus
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=1683368&group_id=5470
Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Python Interpreter Core
Group: Python 2.6
Status: Open
Resolution: None
Priority: 5
Private: No
Submitted By: Blake Ross (blakeross)
Assigned to: Guido van Rossum (gvanrossum)
Summary: object.__init__ shouldn't allow args/kwds
Initial Comment:
object.__init__ currently allows any amount of args and keywords even though
they're ignored. This is inconsistent with other built-ins, like list, that are
stricter about what they'll accept. It's also inconsistent with object.__new__,
which does throw if any are provided (if the default __init__ is to be used).
To reproduce:
object.__init__(object(), foo, bar=3)
This is a slight irritation when using cooperative super calling. I'd like each
class' __init__ to cherry-pick keyword params it accepts and pass the remaining
ones up the chain, but right now I can't rely on object.__init__ to throw if
there are remaining keywords by that point.
----------------------------------------------------------------------
Comment By: Adam Olsen (rhamphoryncus)
Date: 2007-03-21 22:46
Message:
Logged In: YES
user_id=12364
Originator: NO
>> I think it would also help if calling a method via super() didn't
allow
>> positional arguments.
>
> That's absurd, *except* for __init__(), where it could make sense
> depending on the style of cooperation used. But not enough to enforce
this
> in the language; in Py3k you will be able to enforce this on a
per-class
> basis.
The vast majority of "positional" arguments can also be given via name.
The rare exceptions (primarily C functions) may not cooperate well anyway,
so you're trading a relatively obscure limitation for better error
detection.
Perhaps not that important though, since it could be taught as bad style
unless absolutely needed.
>> Having two mixins with the same method name and
>> without a common parent class is just not sane.
>
> Right. This is a cornerstone of cooperative multiple inheritance that
> sometimes seems to be forgotten; there is a big difference between
defining
> a method and extending a method, and only extending methods can make
super
> calls.
>
> The __init__ case is an exception, because there's no requirement that
a
> subclass have a signature compatible with the superclass (if you don't
get
> this, read up on constructors in C++ or Java).
I understand the desire for it to be an exception, I fail to see how it
actually is one. The namespace/signature conflicts exist just the same.
The only way I can see to handle incompatible signatures is to add a flag
that says "I am the *ONLY* class allowed to subclass X" (triggering an
error if violated), have super() entirely bypass it, and then call
X.__init__() directly. Even that doesn't handle X's superclasses being
subclassed more than once, and it looks pretty complicated/obscure anyway.
----------------------------------------------------------------------
Comment By: Guido van Rossum (gvanrossum)
Date: 2007-03-21 22:28
Message:
Logged In: YES
user_id=6380
Originator: NO
> I think it would also help if calling a method via super() didn't allow
> positional arguments.
That's absurd, *except* for __init__(), where it could make sense
depending on the style of cooperation used. But not enough to enforce this
in the language; in Py3k you will be able to enforce this on a per-class
basis.
> Having two mixins with the same method name and
> without a common parent class is just not sane.
Right. This is a cornerstone of cooperative multiple inheritance that
sometimes seems to be forgotten; there is a big difference between defining
a method and extending a method, and only extending methods can make super
calls.
The __init__ case is an exception, because there's no requirement that a
subclass have a signature compatible with the superclass (if you don't get
this, read up on constructors in C++ or Java).
----------------------------------------------------------------------
Comment By: Adam Olsen (rhamphoryncus)
Date: 2007-03-21 18:44
Message:
Logged In: YES
user_id=12364
Originator: NO
I think the avoidance of super() is largely *because* of this kind of
leniency. Consider this snippet on python 2.3:
class Base(object):
def __init__(self, foo=None, *args, **kwargs):
super(Base, self).__init__(foo, *args, **kwargs)
Base(foo='bar')
Base('bar', 42)
Base('bar', 42, x=7)
All pass silently, no error checking. Now consider a python 3.0 version,
with a strict object.__init__:
class Base:
def __init__(self, foo=None, *, y='hi', **kwargs):
super(Base, self).__init__(**kwargs)
Base(foo='bar') # Valid, accepted
Base('bar', 42) # Raises exception
Base('bar', x=7) # Raises exception
The error checking added by this bug/patch and the error checking added by
PEP 3102 (keyword-only arguments) make super a lot more sane.
I think it would also help if calling a method via super() didn't allow
positional arguments. If the base class's arguments can't be given as
keyword args then you probably should call it explicitly, rather than
relying on super()'s MRO.
I was also going to suggest super() should automagically create empty
methods your parent classes don't have, but then I realized you really
should have a base class that asserts the lack of such an automagic
method:
class Base(object):
def mymethod(self, myonlyarg='hello world'):
assert not hasattr(super(Base, self), 'mymethod')
By the time you reach this base class you will have stripped off any extra
arguments that your subclasses added, leaving you with nothing to pass up
(and nothing to pass to). Having two mixins with the same method name and
without a common parent class is just not sane.
----------------------------------------------------------------------
Comment By: Terry J. Reedy (tjreedy)
Date: 2007-03-21 17:16
Message:
Logged In: YES
user_id=593130
Originator: NO
I ask myself, what should I expect from the documentation...
>>> object.__init__.__doc__
'x.__init__(...) initializes x; see x.__class__.__doc__ for signature'
>>> object.__class__
<type 'type'>
>>> type.__doc__
"type(object) -> the object's type\ntype(name, bases, dict) -> a new
type"
and I still don't know ;-).
----------------------------------------------------------------------
Comment By: Blake Ross (blakeross)
Date: 2007-03-21 16:48
Message:
Logged In: YES
user_id=1747060
Originator: YES
Holding the strict version for 3 makes sense to me. Let me know if you
need anything more on my end... thanks for the fast turnaround.
----------------------------------------------------------------------
Comment By: Guido van Rossum (gvanrossum)
Date: 2007-03-21 16:42
Message:
Logged In: YES
user_id=6380
Originator: NO
Well, but since it's been like this for a long time, I don't want to
gratuitously break code. At least not in 2.6. So I'm rejecting the stricter
patch for 2.6. (However, if you want to submit patches that would fix these
breakages anyway, be my guest.)
----------------------------------------------------------------------
Comment By: Blake Ross (blakeross)
Date: 2007-03-21 16:03
Message:
Logged In: YES
user_id=1747060
Originator: YES
Looks good. I skimmed briefly the tests you mentioned. The issue with
test_array appears to be exactly the kind of bug this is intended to
identify: it calls array.__init__(...), but array doesn't have its own
initializer, so object's is used.
I'd guess that the others are failing due to whatever the problem with
pickling is (test_descr uses pickling). I haven't looked into that yet.
I'm sure cooperative super calling of __init__ isn't all that common (it
seems like the mechanism itself isn't used much yet, and may not be until
it's done via keyword) but it doesn't seem like such a bad practice,
especially when mixins are in the picture. There doesn't seem to be a great
alternative.
----------------------------------------------------------------------
Comment By: Guido van Rossum (gvanrossum)
Date: 2007-03-20 15:24
Message:
Logged In: YES
user_id=6380
Originator: NO
I should mention that if we can't get the strict version of this in 2.6,
we should be able to get it into 3.0.
----------------------------------------------------------------------
Comment By: Guido van Rossum (gvanrossum)
Date: 2007-03-20 14:54
Message:
Logged In: YES
user_id=6380
Originator: NO
Here's a stricter version. Unfortunately it breaks a couple of standard
modules; this is a confirmation of my doubts whether the style of
cooperative super calling of __init__ that you use is really the most
common or "best practice".
So far I have only fixed string.py (which would otherwise prevent
extensions from being built); I haven't looked into why the other tests
fail: test_array, test_cpickle, test_descr, test_pickle (and maybe more?).
My conclusion: this would probably break too much code to be worth it. So
I'll have to revert to the previous version. But anyway, here it is for
your perusal.
File Added: new_init_strict.patch
----------------------------------------------------------------------
Comment By: Blake Ross (blakeross)
Date: 2007-03-19 19:27
Message:
Logged In: YES
user_id=1747060
Originator: YES
I think making the check more rigid is a good idea, since this should
throw:
class a(object):
def __init__(self, foo):
super(a, self).__init__(foo)
def __new__(cls, foo):
return object.__new__(cls)
a(1)
(minor typo in the patch: "solution it" -> "solution is")
----------------------------------------------------------------------
Comment By: Guido van Rossum (gvanrossum)
Date: 2007-03-19 17:35
Message:
Logged In: YES
user_id=6380
Originator: NO
This smells enough like a new feature that it couldn't go into 2.5.
----------------------------------------------------------------------
Comment By: Guido van Rossum (gvanrossum)
Date: 2007-03-19 17:31
Message:
Logged In: YES
user_id=6380
Originator: NO
Attached is a patch that implements this proposal, adding copious
commentary. It doesn't seem to break anything in the test suite.
I wonder if we should even make the check more rigid: check the argument
list if either the current method *is* overridden or the other one *is not*
overridden. This would make super calls check the arguments even if the
other method is overridden. What do you think?
File Added: new_init.patch
----------------------------------------------------------------------
Comment By: Blake Ross (blakeross)
Date: 2007-03-19 16:45
Message:
Logged In: YES
user_id=1747060
Originator: YES
Makes sense. I don't think we can ever be completely correct here since
we're inferring intent from the presence of __init__/__new__ that's liable
to be wrong in some cases, but it's likely correct often enough that it's
worth doing.
If I understand correctly, we want to be more forgiving iff one of the two
methods is used, so it seems like we should be complaining if both are used
*or* if neither is used. After all, I could add a __new__ to my coop use
case and I'd still want object to complain. If that's the case, both
object_new and object_init should be complaining if ((tp->tp_new ==
object_new && tp->tp_init == object_init) || (tp->tp_new != object_new &&
tp->tp_init != object_init)).
Of course, for the paranoid, there's always the risk that __new__ will
modify these class functions and change the outcome :) For instance, if a
class had a __new__ and no __init__ and its __new__ changed __new__ back to
object.__new__, object_init on that run would be fooled into thinking it's
using the defaults for both and would complain. I think this could only be
fixed in type_call, which is rather ugly...but then, this *is* a special
case of the "call __init__ after __new__" behavior, and we're trying to
solve it within the methods themselves. Perhaps this last point is academic
enough to be ignored...I don't know why anyone would do this, although the
language makes it possible.
----------------------------------------------------------------------
Comment By: Guido van Rossum (gvanrossum)
Date: 2007-03-19 14:01
Message:
Logged In: YES
user_id=6380
Originator: NO
I'll try to explain why I did it this way. I was considering the single
inheritance case implementing an Immutable object, which overrides __new__
but has no need to override __init__ (since it's too late to do anything in
__init__ for an Immutable object). Since the __init__ still gets called it
would be annoying to have to override it just to make the error go away if
there was a check in __init__. The other case is overriding __init__
without overriding __new__, which is the most common way of doing Mutable
objects; here you wouldn't want __new__ to complain about extra args. So
the only time when you'd want complaints is if both __new__ and __init__
are the defaults, in which case it doesn't really matter whether you
implement this in __init__ or in __new__, so I arbitrarily chose __new__.
I wasn't thinking of your use case at the time though (cooperative super
calls to __init__, which still isn't something I engage in on a day-to-day
basis). I wonder if the right thing to do wouldn't be to implement the
same check both in __init__ and in __new__.
Am I makign sense?
----------------------------------------------------------------------
Comment By: Georg Brandl (gbrandl)
Date: 2007-03-19 02:56
Message:
Logged In: YES
user_id=849994
Originator: NO
I don't really understand either why object_new() checks the arguments,
not object_init():
"""
static int
object_init(PyObject *self, PyObject *args, PyObject *kwds)
{
return 0;
}
/* If we don't have a tp_new for a new-style class, new will use this
one.
Therefore this should take no arguments/keywords. However, this new
may
also be inherited by objects that define a tp_init but no tp_new.
These
objects WILL pass argumets to tp_new, because it gets the same args as
tp_init. So only allow arguments if we aren't using the default init,
in
which case we expect init to handle argument parsing. */
static PyObject *
object_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
{
if (type->tp_init == object_init && (PyTuple_GET_SIZE(args) ||
(kwds && PyDict_Check(kwds) && PyDict_Size(kwds)))) {
PyErr_SetString(PyExc_TypeError,
"default __new__ takes no parameters");
return NULL;
}
return type->tp_alloc(type, 0);
}
"""
----------------------------------------------------------------------
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=1683368&group_id=5470
_______________________________________________
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
