Move generally usable decorators to misc.decorators

[johanrosenkilde]

The decorators in sage.misc.misc should be moved to sage.misc.decorators.

In plot.misc there are also some generally usable decorators for various nice stuff. These should be moved to a general library so other modules can use them without illogically depending on plot. Simultaneously, they should be patched to use sage_wraps instead of the Python "wraps" to accommodate for Sage-specific things; e.g. the fix of Trac #9917.

CC: @jasongrout @mwhansen

Component: relocation

Keywords: generality, decorators

Author: Johan Sebastian Rosenkilde Nielsen

Reviewer: Robert Miller

Merged: sage-4.6.1.alpha2

Issue created by migration from https://trac.sagemath.org/ticket/9907

[johanrosenkilde]

comment:2

While trying to use this patch in Trac #6094, I found out that the @wraps decorator used internally in all three decorators moved to sage.misc only works for functions in Python versions older than 3.2. It was fixed as Python bug issue 3445. Until Sage upgrades to such a new version of Python, I have written a small work-around, which essentially emulates the patched behaviour of wraps. This is in the newest patch.

[rbeezer]

comment:3

I'm not qualified to check out the "work-around" for classes. But I've cc'ed Mike Hansen and Jason Grout, who I think were in on these in the first place back at #4201.

I am running this through full tests as part of #6094 and a report will go there once concluded.

Commit message on the ticket itself should probably change - there is no "as before" here to compare with, and in the final log it really won't make any sense.

[jasongrout]

comment:4

Replying to @johanrosenkilde:

While trying to use this patch in Trac #6094, I found out that the @wraps decorator used internally in all three decorators moved to sage.misc only works for functions in Python versions older than 3.2. It was fixed as Python bug issue 3445. Until Sage upgrades to such a new version of Python, I have written a small work-around, which essentially emulates the patched behaviour of wraps. This is in the newest patch.

Do you think you could just add the work-around to the sage_wraps decorator (sage.misc.misc.sage_wraps) and convert the decorators to use it? There are other functions that use sage_wraps, and it would be nice to have all of that work-around code in one place.

[johanrosenkilde]

comment:5

Replying to @jasongrout:

Replying to @johanrosenkilde:

While trying to use this patch in Trac #6094, I found out that the @wraps decorator used internally in all three decorators moved to sage.misc only works for functions in Python versions older than 3.2. It was fixed as Python bug issue 3445. Until Sage upgrades to such a new version of Python, I have written a small work-around, which essentially emulates the patched behaviour of wraps. This is in the newest patch.

Do you think you could just add the work-around to the sage_wraps decorator (sage.misc.misc.sage_wraps) and convert the decorators to use it? There are other functions that use sage_wraps, and it would be nice to have all of that work-around code in one place.

That should be no problem, I guess; I wasn't aware of sage_wraps. However, that might affect a lot of places outside the original scope of this ticket; should it be a new ticket then? What's the precedence here?

[johanrosenkilde]

comment:6

Replying to @johanrosenkilde:

Replying to @jasongrout:

Replying to @johanrosenkilde:

While trying to use this patch in Trac #6094, I found out that the @wraps decorator used internally in all three decorators moved to sage.misc only works for functions in Python versions older than 3.2. It was fixed as Python bug issue 3445. Until Sage upgrades to such a new version of Python, I have written a small work-around, which essentially emulates the patched behaviour of wraps. This is in the newest patch.

Do you think you could just add the work-around to the sage_wraps decorator (sage.misc.misc.sage_wraps) and convert the decorators to use it? There are other functions that use sage_wraps, and it would be nice to have all of that work-around code in one place.

That should be no problem, I guess; I wasn't aware of sage_wraps. However, that might affect a lot of places outside the original scope of this ticket; should it be a new ticket then? What's the precedence here?

I started Trac #9919 for doing this patch in sage_wraps. I'm also thinking that, when the problems with this patch are identified, we might move sage_wraps and other decorators in sage.misc.misc into sage.misc.decorators as well.

[jasongrout]

comment:7

Starting another ticket sounds good.

[johanrosenkilde]

comment:8

Rob Beezer posted the following problem with the current patch for Trac #6094, but that is actually a problem with the current patch for this trac. When testing the unpickle-function in sage.structure.sage_object, the relocation of the decorators yield problems:

rob@wave:/sage/dev$ ./sage -t  devel/sage/sage/structure/sage_object.pyx
sage -t  "devel/sage/sage/structure/sage_object.pyx"        
**********************************************************************
File "/sage/dev/devel/sage/sage/structure/sage_object.pyx", line 1001:
    sage: print "x"; sage.structure.sage_object.unpickle_all(std)
Expected:
    x...
    Successfully unpickled ... objects.
    Failed to unpickle 0 objects.
Got:
    x
    doctest:1: DeprecationWarning: Your word object is saved in an old file format since FiniteWord_over_OrderedAlphabet is deprecated and will be deleted in a future version of Sage (you can use FiniteWord_list instead). You can re-save your word by typing "word.save(filename)" to ensure that it will load in future versions of Sage.
    doctest:1: DeprecationWarning: Your word object is saved in an old file format since AbstractWord is deprecated and will be deleted in a future version of Sage (you can use FiniteWord_list instead). You can re-save your word by typing "word.save(filename)" to ensure that it will load in future versions of Sage.
    doctest:1: DeprecationWarning: Your word object is saved in an old file format since Word_over_Alphabet is deprecated and will be deleted in a future version of Sage (you can use FiniteWord_list instead). You can re-save your word by typing "word.save(filename)" to ensure that it will load in future versions of Sage.
    doctest:1: DeprecationWarning: Your word object is saved in an old file format since Word_over_OrderedAlphabet is deprecated and will be deleted in a future version of Sage (you can use FiniteWord_list instead). You can re-save your word by typing "word.save(filename)" to ensure that it will load in future versions of Sage.
    doctest:1: DeprecationWarning: ChristoffelWord_Lower is deprecated, use LowerChristoffelWord instead
    ** failed:  _class__sage_plot_misc_options__.sobj
    ** failed:  _class__sage_plot_misc_rename_keyword__.sobj
    Failed:
    _class__sage_plot_misc_options__.sobj
    _class__sage_plot_misc_rename_keyword__.sobj
    Successfully unpickled 584 objects.
    Failed to unpickle 2 objects.
**********************************************************************
1 items had failures:
   1 of   7 in __main__.example_23
***Test Failed*** 1 failures.
For whitespace errors, see the file /home/rob/.sage//tmp/.doctest_sage_object.py
         [5.2 s]
 
----------------------------------------------------------------------
The following tests failed:


        sage -t  "devel/sage/sage/structure/sage_object.pyx"

The deprecation-warnings are just garbage from words.py, so the important thing to note is the "** failed"-lines. The problem, as far as I can see, is simply that there is a standard pickle jar which has a pickle for all the functions etc. shipped with sage. This needs to be updated when applying this patch. Does anyone know how this is done and possibly included in the patch?

Cheers, Johan

[mwhansen]

comment:9

The pickle jar contains old pickles of objects that we want to ensure work in the future. For example, if you made the changes at 9907 as the are, then possibly many pickles out in the wild would break. Instead of trying to update the pickle jar, the appropriate thing to do would be to add the following to sage/plot/misc.py

#For backward compatibility -- see #9907.
from sage.misc.decorators import options, suboptions, rename_keyword

That way, the old pickles will still work as they will still be able to find the decorators in sage.plot.misc.

[johanrosenkilde]

comment:10

Replying to @mwhansen:

The pickle jar contains old pickles of objects that we want to ensure work in the future. For example, if you made the changes at 9907 as the are, then possibly many pickles out in the wild would break. Instead of trying to update the pickle jar, the appropriate thing to do would be to add the following to sage/plot/misc.py

#For backward compatibility -- see #9907.
from sage.misc.decorators import options, suboptions, rename_keyword

That way, the old pickles will still work as they will still be able to find the decorators in sage.plot.misc.

Ok, thanks -- I will update the patch for #9907. I was actually wondering a bit about backward compatibility, so good to know this is the system. Does the above fix then have a one-year lifetime like for keyword-deprecation? If so, should it be flagged somehow (e.g. with the comment containing the term "backward compatibility") so it can be found and removed one year from now?

[johanrosenkilde]

comment:14

Note that the added patch assumes the patch for Trac #9919.

[johanrosenkilde]

comment:15

Added a comment about when then bug in #9919 was fixed in Python. Accidentally uploaded two.

[johanrosenkilde]

comment:16

Ok, uploaded a new one, as it seems you can't manually change the patch-files because of some hashes in the top of the file. So this one was done the right way. Remember to use the newest and
NOT the one called ".2.patch".

[jasongrout]

comment:17

Can I delete the .2.patch file, then?

[jasongrout]

comment:18

I've opened up #10057 to address an import change that needs to be made in sagenb after this is merged.

[jasongrout]

apply only this patch; rebased to 4.6.alpha1+some patches

[johanrosenkilde]

comment:19

Attachment: trac_9907_move_decorators.patch.gz

Replying to @jasongrout:

Can I delete the .2.patch file, then?

Sure; I just don't have the privileges to do that myself.

[mwhansen]

comment:20

This needs to be coordinated with #10107.

[johanrosenkilde]

Attachment: trac_9907_move_decorators.2.patch.gz

Rebased to 4.6. Still requires first applying patch for Trac #9919

[rlmill]

Reviewer: Robert Miller

[rlmill]

comment:21

If I apply #6094, #9907, #9919, and #10107 together on top of sage-4.6, all long tests pass. The code looks good.

[jdemeyer]

comment:22

Should this be merged in Sage 4.6.1? (if yes, please set the Milestone)

[jdemeyer]

Changed author from jsrn to Johan Sebastian Rosenkilde Nielsen

[jdemeyer]

Merged: sage-4.6.1.alpha2