Replace sage_wraps by decorator library

[tobiasdiez]

Replace the sage custom sage_wrap and decorator_defaults methods by the well-maintained decorator library https://github.com/micheles/decorator.

My motivation is to replace sage's autodoc sphinx extension with the built-in one. For this, the Phyton introspection code needs to work without sage's customization. This ticket is a first step towards this goal. But independently of this motivation the approach using decorator library already provides cleaner code and better introspection results with respect to annotations (e.g. typing information). See the following test script for a comparison:

import inspect
from pathlib import Path
from sage.misc.decorators import decorator_keywords, sage_wraps
from decorator import decorator, getfullargspec
from sage.misc.sageinspect import sage_getargspec

@decorator
def warn_slow(func, timelimit=60, *args, **kw):
    print('%s took %d seconds', func.__name__, timelimit)
    return func(*args, **kw)

@warn_slow
def preprocess_input_files(inputdir: Path, tempdir):
    pass

@warn_slow(timelimit=600)
def run_calculation(tempdir, outdir):
    pass

print("With decorator")
print(getfullargspec(preprocess_input_files))
# FullArgSpec(args=['inputdir', 'tempdir'], varargs=None, varkw=None, defaults=None, kwonlyargs=[], kwonlydefaults=None, annotations={'inputdir': <class 'pathlib.Path'>})
print(inspect.getfullargspec(preprocess_input_files))
# FullArgSpec(args=['inputdir', 'tempdir'], varargs=None, varkw=None, defaults=None, kwonlyargs=[], kwonlydefaults=None, annotations={'inputdir': <class 'pathlib.Path'>})
# !! Yields full information as expected
print(sage_getargspec(preprocess_input_files))
# ArgSpec(args=['inputdir', 'tempdir'], varargs=None, keywords=None, defaults=None)
# !! No annotation information
print(getfullargspec(run_calculation))
# FullArgSpec(args=['tempdir', 'outdir'], varargs=None, varkw=None, defaults=None, kwonlyargs=[], kwonlydefaults=None, annotations={})
print(inspect.getfullargspec(run_calculation))
# FullArgSpec(args=['tempdir', 'outdir'], varargs=None, varkw=None, defaults=None, kwonlyargs=[], kwonlydefaults=None, annotations={})
print(sage_getargspec(preprocess_input_files))
# ArgSpec(args=['inputdir', 'tempdir'], varargs=None, keywords=None, defaults=None)

@decorator_keywords
def warn_slow_sage(func=None, timelimit=60):
     @sage_wraps(func)
     def wrapper(*args, **kwargs):
         print('%s took %d seconds', func.__name__, timelimit)
         return func(*args, **kw)
     return wrapper

@warn_slow_sage
def preprocess_input_files_sage(inputdir: Path, tempdir):
    pass

@warn_slow_sage(timelimit=600)
def run_calculation_sage(tempdir, outdir):
    pass

print("With sage")
print(getfullargspec(preprocess_input_files_sage))
# FullArgSpec(args=[], varargs='args', varkw='kwargs', defaults=None, kwonlyargs=[], kwonlydefaults=None, annotations={})
print(inspect.getfullargspec(preprocess_input_files_sage))
# FullArgSpec(args=[], varargs='args', varkw='kwargs', defaults=None, kwonlyargs=[], kwonlydefaults=None, annotations={})
# !! No args nor annotation
print(sage_getargspec(preprocess_input_files_sage))
# ArgSpec(args=['inputdir', 'tempdir'], varargs=None, keywords=None, defaults=None)
print(getfullargspec(run_calculation_sage))
# !! No annotation info
# FullArgSpec(args=[], varargs='args', varkw='kwargs', defaults=None, kwonlyargs=[], kwonlydefaults=None, annotations={})
print(inspect.getfullargspec(run_calculation_sage))
# FullArgSpec(args=[], varargs='args', varkw='kwargs', defaults=None, kwonlyargs=[], kwonlydefaults=None, annotations={})
print(sage_getargspec(preprocess_input_files_sage))
# ArgSpec(args=['inputdir', 'tempdir'], varargs=None, keywords=None, defaults=None)

Replacing sage_getargspec with inspect.getfullargspec will be done in a follow-up ticket (as this is already big enough).

Todo (as follow-up): Migrate cachefunction etc to decorator framework. The documentation https://github.com/micheles/decorator/blob/master/docs/documentation.md#the-solution contains an example (memoize) of how this may look like.

Depends on #26254

CC: @mkoeppe @kwankyu

Component: doctest framework

Work Issues: Fix patchbot warnings

Author: Tobias Diez

Branch/Commit: public/documentation/replaceDecorator @ 1b656a3

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

[sagetrac-git]

Changed commit from b414b8e to dbe456a

[sagetrac-git]

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

dbe456a

Remove unused decorator_defaults

[sagetrac-git]

Changed commit from dbe456a to 0547957

[sagetrac-git]

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

0547957

Fix stupid mistakes

[mkoeppe]

comment:3

Functions such as sage_wraps cannot just be removed, as they may be used by user code. They need to go through deprecation

[sagetrac-git]

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

3a433f7	Fix option wrappers
8b05582	Readd unused functions, but deprecated them

[sagetrac-git]

Changed commit from 0547957 to 8b05582

[tobiasdiez]

comment:5

Good to know! I've readded and marked as deprecated them.

This should be now ready for review.

[tobiasdiez]

Changed work issues from Fix patchbot warnings to none

[mkoeppe]

comment:8

Better squash changes like this so that git blame is more accurate

[mkoeppe]

comment:9

Does the decorator library really misspell decorator as decorater?

[sagetrac-git]

Changed commit from 8b05582 to 2c781d9

[sagetrac-git]

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

2c781d9

Fix imports

[sagetrac-git]

Changed commit from 2c781d9 to 3f04a50

[sagetrac-git]

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

3f04a50

Fix spelling

[tobiasdiez]

comment:12

Replying to @mkoeppe:

Does the decorator library really misspell decorator as decorater?

No of course not. That was a result of some last minute changes on my side. Fixed now.

---

New commits:

3f04a50

Fix spelling

[tobiasdiez]

Work Issues: Fix patchbot warnings

[jhpalmieri]

comment:15

Can you explain the purpose of the change

diff --git a/src/sage_setup/docbuild/__init__.py b/src/sage_setup/docbuild/__init__.py
index b07e9c1..a4ba68e 100644
--- a/src/sage_setup/docbuild/__init__.py
+++ b/src/sage_setup/docbuild/__init__.py
@@ -564,7 +564,7 @@ class ReferenceBuilder(AllBuilder):
             if not os.path.exists(refdir):
                 continue
             logger.info('Building bibliography')
-            self._build_bibliography(lang, format, *args, **kwds)
+            #self._build_bibliography(lang, format, *args, **kwds)
             logger.info('Bibliography finished, building dependent manuals')
             self._build_everything_except_bibliography(lang, format, *args, **kwds)
             # The html refman must be build at the end to ensure correct

?

[sagetrac-git]

Changed commit from 3f04a50 to f7c1b7e

[mkoeppe]

comment:23

What's the status of this ticket? Just came across a sage_getargspec bug (#31309), so it would be great if we can get rid of it

[sagetrac-git]

Changed commit from fc23e73 to b4f05c7

[sagetrac-git]

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

99002b9	Merge branch 'develop' of git://github.com/sagemath/sage into public/documentation/replaceDecorator
b4f05c7	Fix compile error

[tobiasdiez]

comment:25

It is working in principle, but I still need to fix a few things here and there. The biggest problem is that there are some changes to cython files needed, for which it would be really handy if we could get #30371 merged.

[mkoeppe]

comment:26

Setting new milestone based on a cursory review of ticket status, priority, and last modification date.

[mkoeppe]

comment:27

Setting a new milestone for this ticket based on a cursory review.

[sagetrac-git]

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

73c6544	Merge branch 'develop' of https://github.com/sagemath/sage into public/documentation/replaceDecorator
a20f208	Fix a few things

[sagetrac-git]

Changed commit from b4f05c7 to a20f208

[tobiasdiez]

Changed work issues from Fix patchbot warnings to Wait for micheles/decorator#137 to be merged and released, update decorator lib, x patchbot warnings

[tobiasdiez]

Dependencies: #26254

[tobiasdiez]

Changed work issues from Wait for micheles/decorator#137 to be merged and released, update decorator lib, x patchbot warnings to Fix patchbot warnings

[sagetrac-git]

Changed commit from a20f208 to 89487a7

[sagetrac-git]

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

5751b7d	Merge branch 'develop' of https://github.com/sagemath/sage into public/documentation/replaceDecorator
ca86cc9	build/pkgs/decorator: Update to 5.1.1
146bc0b	Revert changes to cache functions
bff4f3a	Replace deprecated decorator by message
e3d5bc0	Fix deprecation wrapper
89487a7	Activate binding=true compiler directive

[sagetrac-git]

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

6adf9da	Readd empty line
1b656a3	Readd sage_wraps in deprecated methods

[sagetrac-git]

Changed commit from 89487a7 to 1b656a3