On Saturday, 7 December 2013 05:57:36 UTC+1, Travis Scrimshaw wrote: > > Hey everyone, > The following gives a PicklingError: > > sage: P = Partitions(5, max_length=4) > sage: loads(dumps(P)) >
Hi Travis, As you say, the error occurs in pickling so it actually happens in the dumps(P) call. I'm sure you know this, so this remark is for anyone else who cares:) The actual object created in an instance of `IntegerListsLex`, and the > pickling error is due to the fact that the global options > `PartitionOptions` cannot pickled since it passes lambda functions as > parameters. The ParittionOptions cannot be pickled is okay IMO since there > is a unique such global object in Sage. However, this causes problems when > creating partitions with constraints (for my purposes, I need all > partitions of a bounded max length). The solution is to create a new class > which has class attributes at least for the global options, in fact I'd > also give it an `Element` as well (and perhaps inherit from `Partitions` -- > I haven't tried that yet to see if things break): > As GlobalOptions are not pickeable I agree that this error probably occurs because of PartitionOptions being passed as an argument to the IntegerListsLex class. Unfortunately, I think that this is the real problem: global options for one class are not intended to be passed as arguments to another class. Global options are meant to deal with only superficial aspects of a class such as how they are printed or latexed. These options should not contain any data which is intrinsic to any instance of the class, so pickling should not depend on them. In the code for IntegerListsLex it appears that the global_options argument to IntegerListsLex may have been added relatively recently because this argument is current *undocumented* and, moreover, it is *not used* anywhere in the class (in other words, we already have at least a documentation bug). I think that adding a global_option argument to IntergerListLex was a mistake and should be reversed. *I would argue that this is self-evident because whenever this feature is used the resulting objects will not pickle.* In particular, this problem is larger than just the example you give above: searching through partition.py reveals several other instances and there may well be other examples in other files. I suggest that the correct way out of this pickle is to remove the global_options argument from IntegeListLex and rewrite any code that uses this "feature". Probably the easiest way to do this, given that this happens many times in partition.py, is to make a new class, say PartitionsIntegerListLex, which inherits from IntegerListLex and then overrides any method which uses PartitionOptions. If you look in my labelled matrix <http://trac.sagemath.org/ticket/14103>patch you'll see something similar going on, but more in the opposite direction. In this case when a labelled matrix which has rows and/or columns indexed by partitions then options in the matrix are used to pick out the optionable methods of partitions but it does this without changing PartitionOptions. The point is the same, however, in that the global options for one class are local to one class and they should not be relied upon, or passed to, another class. In any case, I feel quite strongly that the global_options argument in IntegerListsLex should be removed and another solution found. Just in case the general consensus disagrees with me (democracy has its' problems after all:), I'll also give my answers to Travis' other questions. > class Partitions_constraints_new(IntegerListsLex): > Element = Partition > global_options = _Partitions.global_options > > So I have a few questions about this right now. Do we want to still > keep backwards compatible unpickling of (very) old `Partitions_constraints` > (below sage-3.4.1), and if so, what should we call this new class? Do we > want to be able to pass things like NN to `Partitions` (and use this class) > and/or continue to allow `IntegerListsLex` to vary over a range of n? > Currently we can pass NN to Partitions or, equivalently None. I think it makes sense as it returnsthe class of all Partitions which is sometimes useful. I don't know anything about IntegerListsLex, so I can't comment there, but I would like to keep NN as a possible argument to Partitions. Regarding old pickles I have nothing against removing them, although I suspect that if you ask this question on sage-dev then you might meet with some resistance. In any case, it is not necessary to remove them: you can simply rewrite __getstate__ and __setstate__ so that they use a version number (no version number defaults to the old version). By doing this it should be possible to have both the old and new pickles happily living side by side on the self. This way, even though at least one pickle may be mouldy after a few years, they should still unpickle. If you do follow this route, though, can you please find a more informative class name than Partitions_constraints_new. A part from this name not telling me anything meaningful, the name will not be appropriate in a few years time. Andrew -- You received this message because you are subscribed to the Google Groups "sage-combinat-devel" group. To unsubscribe from this group and stop receiving emails from it, send an email to sage-combinat-devel+unsubscr...@googlegroups.com. To post to this group, send email to sage-combinat-devel@googlegroups.com. Visit this group at http://groups.google.com/group/sage-combinat-devel. For more options, visit https://groups.google.com/groups/opt_out.
