Pickling and otherwise enhancing global options

[andrew.mathas]

Instances of the ​GlobalOptions class in sage.structure.global_options do not pickle, which is annoying and causes various problems. (This is a defect.)

In addition, when the GlobalOptions class was introduced it was suggested in ​sage-dev that it "would be nice" if we could implement the syntax used in the IPython configuration:

    A.options.foobar = 1

in addition to

    A.options(foobar=1)

This ticket implements both of these features and, in addition, makes it possible to construct options classes dynamically.

As was also suggested on sage-dev, I have also renamed all of the global_options methods simply as options, with the global_options variants being deprecated. The "stand-alone" options classes such as PartitionOoptions, TableauxOptions, ... have also been deprecated and, instead, put inside their "parent" classes where they are accessible using the options method (global_options methods have been deprecated). This is cleaned up the code a little and it seems to speed up the sage start-up time.

The pickling is done by adding an extra module argument to the GlobalOptions that specifies the module which contains the class that the options are attached -- name of of the class defaults to the name of the options class but this can be explicitly set using the options_class argument. The options class is assumed to have an options method, and this is used to unpickle a pickle for an options class. GlobalOptions that do not arise as options methods for standard sage classes cannot be unpickled. The reason why the name of the module and class are passed, as strings, to GlobalOptions rather than the actual class is that this approach allows the options to be constructed during the initialisation of the (instance of the) class and, in turn, this allows the class to dynamically construct their options classes.

[andrew.mathas]

As I am cleaning this up a related question comes to mind: the name GlobalOptions is slightly unfortunate because instances of this class are really just options for an associated family of objects. I can sort of forgive myself for using this name for the class, but in all of the instances of this class the associated "parent" classes have a global_options method that points back to its options class. This method is now being enshrined more firmly inside the code because it is exploited to allow pickling. I would be happier if these methods were simply called options, rather than global_options, even though this change would necessitate deprecating the existing global_options methods.

Thoughts?

[andrew.mathas]

New commits:

​f006a84	Adding pickling methods
​2ba392f	Initial version - some failing doctests

[git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

​3a4a9d2	20618: imported sage.modules.tutorial_free_modules and thematic_tutorials/tutorial-implementing-algebraic-structures from the Sage-Combinat queue
​3e3e350	20618: fixed missing file
​500d2d8	Global options to options
​52d7cf6	Recasting options as attributes
​64d0209	Global options as class attributes
​25ab72f	Fixing doctests
​23f2c56	Merging into 7.3.beta4
​12cb846	Fixing more doc-tests

[git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

​c676864	Global options to options
​9313ce8	Recasting options as attributes
​11a13f4	Global options as class attributes
​217b5b4	Fixing doctests
​b761fec	Merging into 7.3.beta4
​caabdb0	Fixing more doc-tests
​11b7080	More doc-tests

[andrew.mathas]

Replying to git:

This was to remove 20618 from the commit tree as it shouldn't have been there...

[tscrim]

Okay, I fixed the doctest failures with permutation.py. The issue was that the _Option (now Option so it appears in the doc) did not define a __ne__ method, and so it returned True for things like opt != X even when opt == X.

The other doctest failures were due to lack of conversion to options from global_options.

This is a very nice improvement over the current interface to (global) options. If the doctest pass for you and my changes are good, then you can set this to a positive review.

---

New commits:

​0636c87	Merge branch 'u/andrew.mathas/pickling_global_options' of trac.sagemath.org:sage into public/misc/pickling_global_options-18555
​9642057	Making everything work with the new API.
​f42beb4	Rename _Option to Option and some PEP8 to global_options.py.

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​4013abb

Fixing failing doctests identified by the patchbot

[tscrim]

There's some doc issues in the manifold.py options: EXAMPLES:: and everything including and after sage: latex(f) is over indented.

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​3f5e0e9

Fixing manifold documemtation issues and collecting all deprecation messages at the end of the files

[andrew.mathas]

Thanks Travis. Fixed. I have also cleaned up the deprecation messages, putting them in the same place, so that they will be easier to remove in future.

Finally, should the reset method be _reset() or Reset()? Currently it is _reset() but it probably shouldn't be hidden.

[tscrim]

Reset() breaks python conventions, so that is a -1 from me. I think reset() or _reset() are both fine. Yet there is not so much a good general reason to go back to defaults other than for testing purposes in my mind. Hence the hidden _reset() is better IMO.

One last thing, you added an __init__ method in interval_poset.py. Is this necessary? If so, it needs a doctest. Once this is done, this will be at a positive review.

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​8322fd5

Removing __init__ from interval_poset.py

[andrew.mathas]

Fixed -- not sure how that got in there. Thanks Travis!

[vbraun]

Reviewer name is missing

[vbraun]

Merge conflict, try next beta

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​ed5070d

Merging 7.3.beta7

[andrew.mathas]

Merged into 7.3.beta7. Tests pass.

[vbraun]

Fails on 32-bit

sage -t --long src/sage/structure/global_options.py
**********************************************************************
File "src/sage/structure/global_options.py", line 599, in sage.structure.global_options.Option.__hash__
Failed example:
    hash( Tableaux.options.convention ) # indirect doc-test
Expected:
    -6673246059928475433
Got:
    -1691342633
**********************************************************************
1 item had failures:
   1 of   2 in sage.structure.global_options.Option.__hash__
    [135 tests, 1 failure, 0.03 s]

[tscrim]

This just needs to mark the different output as # 32-bit and # 64-bit, but I'd change the actual test to hash(Tableaux.options.convention) == hash(Tableaux.options('convention') (it also doesn't need the # indirect doctest marker). Sorry for not catching that sooner.

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​ba7dd98

Fixing hashed doct-test

[andrew.mathas]

Replying to tscrim:

This just needs to mark the different output as # 32-bit and # 64-bit, but I'd change the actual test to hash(Tableaux.options.convention) == hash(Tableaux.options('convention') (it also doesn't need the # indirect doctest marker). Sorry for not catching that sooner.

Thanks Travis! Fixed and changed back to a positive review.