Private attribute from category?

[Simon King]

Hi!

At http://groups.google.com/group/sage-devel/browse_thread/thread/b730edf54d357e3b
I asked about attribute lookup of elements and parents. It did not get
a reply. Since it is related with categories, I ask here:

I learned that, in Python, attribute names starting with two
underscores and not ending with an underscore are "private", hence,
the convention is that they belong to a particular instance and not to
its class.

Currently, the attribute lookup for Parent and Element does not care
about that convention:
sage: QQ.category().parent_class.__foo = 'bar'
sage: QQ.__foo
'bar'

Question:
Should we obey the convention? I.e., should the __getattr__ method of
Parent and Element immediately raise an error if a private attribute
is requested, so that they are not looked up in the parent or element
classes of the category?

My vote is "Yes", for the following reasons:

* The convention actually is obeyed by the Sage developers: All
doctests still pass if one does the change.

* Private attributes are typically used to store some data that are
computed once and are very often requested afterwards. Example from
SageObject.__repr__():
if hasattr(self, '__custom_name'):
return self.__custom_name

Since it is clear that attributes like '__custom_name' will not be
defined in the category, it would be a waste of time to try and look
it up there.

If you agree then please review #10467 ! If you disagree then please
post at the ticket (or here) as well.

Cheers,
Simon

[Nicolas M. Thiery]

Hi Simon,

On Mon, Dec 13, 2010 at 03:05:22AM -0800, Simon King wrote:
> At http://groups.google.com/group/sage-devel/browse_thread/thread/b730edf54d357e3b
> I asked about attribute lookup of elements and parents. It did not get
> a reply. Since it is related with categories, I ask here:

Thanks for the prompting! I am not able to follow sage-devel currently.

I agree with your e-mail. Just please make a quick benchmark (and post
the result on track) to verify that the extra check does not induce an
overhead for usual lookups (probably not). I'll try to review this
during the holidays, but it would be great if someone could take up
the task to get it done sooner.

Cheers,
Nicolas

> I learned that, in Python, attribute names starting with two
> underscores and not ending with an underscore are "private", hence,
> the convention is that they belong to a particular instance and not to
> its class.
>
> Currently, the attribute lookup for Parent and Element does not care
> about that convention:

Just a precision: for Parents and Elements implemented in Cython.

> sage: QQ.category().parent_class.__foo = 'bar'
> sage: QQ.__foo
> 'bar'
>
> Question:
> Should we obey the convention? I.e., should the __getattr__ method of
> Parent and Element immediately raise an error if a private attribute
> is requested, so that they are not looked up in the parent or element
> classes of the category?
>
> My vote is "Yes", for the following reasons:
>
> * The convention actually is obeyed by the Sage developers: All
> doctests still pass if one does the change.
>
> * Private attributes are typically used to store some data that are
> computed once and are very often requested afterwards. Example from
> SageObject.__repr__():
> if hasattr(self, '__custom_name'):
> return self.__custom_name
>
> Since it is clear that attributes like '__custom_name' will not be
> defined in the category, it would be a waste of time to try and look
> it up there.
>
> If you agree then please review #10467 ! If you disagree then please
> post at the ticket (or here) as well.
>
> Cheers,
> Simon

Nicolas
--
Nicolas M. Thi�ry "Isil" <nth...@users.sf.net>
http://Nicolas.Thiery.name/

[Simon King]

Hi Nicolas,

Thanks for the feedback!

On 13 Dez., 12:58, "Nicolas M. Thiery" <Nicolas.Thi...@u-psud.fr>
wrote:

> I agree with your e-mail. Just please make a quick benchmark (and post
> the result on track) to verify that the extra check does not induce an
> overhead for usual lookups (probably not).

I just posted it on track, but I hope you don't mind that I repeat it
here, for completeness.

I thought that there is one situation where my patch provides a
disadvantage: Namely when an attribute is requested that is not
defined by the instance or its class, but is defined by the category.
An example is the method `summation` of QQ:
sage: QQ.summation(1,2)
3
sage: QQ.summation.__module__
'sage.categories.commutative_additive_semigroups'

But instead of the expected small deceleration, the attribute lookup
seems even quicker as before (I don't know why, though):
Without patch:
{{{
sage: timeit('a=QQ.summation',number=50000)
50000 loops, best of 3: 553 ns per loop
sage: timeit('a=QQ.summation',number=50000)
50000 loops, best of 3: 563 ns per loop
sage: timeit('a=QQ.summation',number=50000)
50000 loops, best of 3: 559 ns per loop
}}}

With patch
{{{
sage: timeit('a=QQ.summation',number=50000)
50000 loops, best of 3: 530 ns per loop
sage: timeit('a=QQ.summation',number=50000)
50000 loops, best of 3: 514 ns per loop
sage: timeit('a=QQ.summation',number=50000)
50000 loops, best of 3: 539 ns per loop
}}}

Chers,
Simon

[Simon King]

PS:

On 13 Dez., 12:58, "Nicolas M. Thiery" <Nicolas.Thi...@u-psud.fr>
wrote:

> > Currently, the attribute lookup for Parent and Element does not care
> > about that convention:
>
> Just a precision: for Parents and Elements implemented in Cython.

By "Parent" and "Element", I actually meant the (indeed Cython)
classes sage.structure.parent.Parent and
sage.structure.element.Element.

Cheers,
Simon