Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#4371 closed enhancement (fixed)

[with patch, positive review] Add support for lazy attributes via a decorator

Reported by: nthiery Owned by: nthiery
Priority: major Milestone: sage-3.3
Component: misc Keywords:
Cc: Merged in:
Authors: Reviewers:
Report Upstream: Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description

The lazy-attribute-...-nt patch on the sage-combinat patch servers is a first implementation of lazy attributes (see the doc in the patch). Comments and alternative implementations welcome! See the current caveats in the doc

Cheers,

Nicolas

Attachments (2)

lazy_attributes-4371-nt.patch (5.8 KB) - added by nthiery 11 years ago.
Patch
lazy_attributes-4371-nt.2.patch (8.5 KB) - added by nthiery 11 years ago.

Download all attachments as: .zip

Change History (16)

Changed 11 years ago by nthiery

Patch

comment:1 follow-up: Changed 11 years ago by nthiery

  • Summary changed from Add support for lazy attributes via a decorator to Add support for lazy attributes via a decorator (with preliminary patch)

In case I did not make myself clear: this patch is *not* ready for insertion in sage. It's more of a request for comments!

comment:2 in reply to: ↑ 1 Changed 11 years ago by mvngu

Replying to nthiery:

In case I did not make myself clear: this patch is *not* ready for insertion in sage. It's more of a request for comments!


Here are some possible fixes for improving the documentation of the patch lazy_attributes-4371-nt.patch:

1.

-easilly override a given attribute; you don't need to call the
+easily override a given attribute; you don't need to call the


2.

-the internal dictionnary of the object:
+the internal dictionary of the object:


Otherwise, the doc looks OK to me.

comment:3 follow-up: Changed 11 years ago by nthiery

Fixes applied. Thanks. Lazy attributes now work well with new style classes:

http://sage.math.washington.edu:2144/file/03b4130fb25d/lazy_attributes-4371-nt.patch

comment:4 in reply to: ↑ 3 Changed 11 years ago by mvngu

Replying to nthiery:

Fixes applied. Thanks. Lazy attributes now work well with new style classes:

http://sage.math.washington.edu:2144/file/03b4130fb25d/lazy_attributes-4371-nt.patch


For your new patch at the above URL, here's a fix to improve your documentation:

-Invoking Descriptors in the python reference manual).
+Invoking Descriptors in the Python reference manual).

comment:5 Changed 11 years ago by mabshoff

  • Summary changed from Add support for lazy attributes via a decorator (with preliminary patch) to [with patch, needs review] Add support for lazy attributes via a decorator

Nicolas,

should this patch be reviewed?

Cheers,

Michael

comment:6 Changed 11 years ago by nthiery

  • Owner changed from cwitty to nthiery
  • Status changed from new to assigned

Comments on it are more than welcome (in particular for the hasattr feature). No need to waste time on a complete final review though.

Thanks,

Nicolas

comment:7 follow-up: Changed 11 years ago by mhansen

  • Summary changed from [with patch, needs review] Add support for lazy attributes via a decorator to [with patch, not ready for review] Add support for lazy attributes via a decorator

comment:8 in reply to: ↑ 7 Changed 11 years ago by nthiery

  • Summary changed from [with patch, not ready for review] Add support for lazy attributes via a decorator to [with patch, needs review] Add support for lazy attributes via a decorator

Ready for review. Not all desired features are implemented, but we need the bulk for upcoming patches.

Changed 11 years ago by nthiery

comment:9 Changed 11 years ago by hivert

  • Milestone changed from sage-combinat to sage-3.3

comment:10 Changed 11 years ago by rlm

  • Summary changed from [with patch, needs review] Add support for lazy attributes via a decorator to [with patch, positive review] Add support for lazy attributes via a decorator

comment:11 Changed 11 years ago by rlm

Apply lazy_attributes-4371-nt.2.patch only.

comment:12 Changed 11 years ago by mabshoff

For the record note that

  • __init___
  • __get__

do not have doctests, but this will not stop the merge in this case :)

Cheers,

Michael

comment:13 Changed 11 years ago by mabshoff

  • Resolution set to fixed
  • Status changed from assigned to closed

Merged lazy_attributes-4371-nt.2.patch in Sage 3.3.rc1.

Cheers,

Michael

comment:14 Changed 11 years ago by nthiery

Thanks Michael and Robert.

Michael: could you please fold the patch into sage-3.3 on the sage-combinat patch server?

Thanks!

Note: See TracTickets for help on using tickets.