Opened 8 years ago

Last modified 6 years ago

#13811 needs_work defect

LazyFamily cannot be copied if it can't be pickled

Reported by: cnassau Owned by: was
Priority: minor Milestone: sage-6.4
Component: pickling Keywords: LazyFamily, copy, pickle
Cc: Merged in:
Authors: Christian Nassau Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

Currently, copying of LazyFamily objects only works for families that can be pickled:

   sage: def unpicklableFamily():
   ...       x = 10
   ...       return LazyFamily([1,2,3], lambda n: x*n)
   sage: f = unpicklableFamily() 
   sage: copy(f)
   Traceback (most recent call last):
   ...
   ValueError: Cannot pickle code objects from closures

I suggest adding a LazyFamily.__copy__ method that fixes this.

Attachments (1)

lazyfamcopy.patch (1.7 KB) - added by cnassau 8 years ago.
A patch for this problem

Download all attachments as: .zip

Change History (12)

comment:1 Changed 8 years ago by cnassau

  • Status changed from new to needs_review

comment:2 Changed 8 years ago by cnassau

  • Authors set to cnassau

comment:3 Changed 8 years ago by cnassau

  • Authors changed from cnassau to Christian Nassau

comment:4 follow-up: Changed 8 years ago by nthiery

Hi Christian!

Families are (semantically) immutable objects. Why would we want to copy them?

Cheers,

Nicolas

comment:5 in reply to: ↑ 4 ; follow-up: Changed 8 years ago by cnassau

Replying to nthiery:

Hi Christian!

Families are (semantically) immutable objects. Why would we want to copy them?

Good point... I stumbled across this issue while I was struggling to create a disjoint union of a dynamically generated family of LazyFamily objects, while the real problem was that I was using closures with reference to a loop variable. I eventually resolved this issue, and my solution does not requiry any copies now. So I agree that there's no need to create shallow copies.

I do think that user code can expect that copy(any_sage_object) does not throw an error. Maybe LazyFamily.__copy__(self) should just return self? This would be in accordance with

sage: copy(Integers()) is Integers()
True

Cheers, Christian

comment:6 in reply to: ↑ 5 Changed 8 years ago by nthiery

Replying to cnassau:

I do think that user code can expect that copy(any_sage_object) does not throw an error. Maybe LazyFamily.__copy__(self) should just return self? This would be in accordance with

sage: copy(Integers()) is Integers()
True

Yes, it would make sense to have copy(x) return x for all immutable objects in Sage. I am not sure how to achieve this though: we do not (yet?) have a class for providing code for all immutable objects in Sage, and I would not want to force every relevant class to reimplement a trivial copy method.

For the case at hand (families), maybe one could add a method Parent.copy returning self? As far as I can tell, parents are required to be immutable anyway. But one should double check this.

Cheers,

Nicolas

Changed 8 years ago by cnassau

A patch for this problem

comment:7 Changed 8 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:8 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:9 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:10 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:11 Changed 6 years ago by vdelecroix

  • Status changed from needs_review to needs_work

Hi,

As Nicolas suggested in his comnent 6, __copy__ should be implemented as

def __copy__(self):
    return self

It is standard Python for immutable objects

sage: t = (1,2,3)
sage: copy(t) is t
True
sage: i = 1231491283r
sage: copy(i) is i
True 

Vincent

Note: See TracTickets for help on using tickets.