Upgrade sagenb

[jasongrout]

Upgrade to the latest version of sagenb. Tarball for upstream/ at http://sage.math.washington.edu/home/kcrisman/sagenb-0.11.2.tar

One useful fix is the fixing of various import locations: sagemath/sagenb#280
We deprecate for real all these fixed sagenb imports in #17460.

This will also fix #11275, #8738, #8427, #17490.

See also #17482 and #17515 for Sage issues related to this update.

Upstream: Fixed upstream, in a later stable release.

CC: @johanrosenkilde

Component: packages: standard

Keywords: sd35.5

Author: Karl-Dieter Crisman

Branch/Commit: 14d3291

Reviewer: Jeroen Demeyer

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

[mwhansen]

comment:2

Attachment: trac_10057.patch.gz

[mwhansen]

Author: Mike Hansen

[zimmermann6]

comment:3

Mike,

I started to review that ticket, but I'm puzzled since

sage: from sage.misc.misc import decorator_defaults

still works. Shouldn't it fail?

Also, shouldn't the patch include a doctest?

Paul

[zimmermann6]

Changed keywords from none to sd35.5

[zimmermann6]

Reviewer: Paul Zimmermann

[johanrosenkilde]

comment:4

Replying to @zimmermann6:

Mike,

I started to review that ticket, but I'm puzzled since

sage: from sage.misc.misc import decorator_defaults

still works. Shouldn't it fail?

The reason it doesn't fail is because sage.misc.misc contains an import statement, importing decorator_defaults among other (line 2735). This was introduced in #9907 for backwards compatibility, in case users assumed that these decorators would still be in sage.misc.misc. I assumed this would be a temporary thing, just as for keyword and attribute changes. #9907 was merged 14 months ago, so this is longer than the deprecation lifetime, but on the other hand, no deprecation warning has been issued to users doing the wrong thing for these past 14 months. I'm not sure what is usually done under such circumstances. Also, removing the import statement from sage.misc.misc will result in the pickle jar breaking, and I'm not sure how to remedy that and what consequences it might have. Need expert opinion here.

Also, shouldn't the patch include a doctest?

Maybe that's just me, but isn't it a bit too much including a doctest just for a relocation of a function? It's still the same function.

[zimmermann6]

comment:5

ok I understand. But then I don't know what a reviewer is supposed to do for such a patch.
Anyway I put it back to needs review.

Paul

[jdemeyer]

Changed reviewer from Paul Zimmermann to Mike Hansen, Paul Zimmermann

[jdemeyer]

Changed author from Mike Hansen to none

[jdemeyer]

comment:7

I'm working on a sagenb pull request.

[kcrisman]

Upstream: Not yet reported upstream; Will do shortly.

[kcrisman]

comment:8

Hey, only the release manager gets to close a ticket! ;-)

More seriously, should we remove the 'deprecated' import or is the pickle jar issue mentioned above serious enough to not do that? Anyway, better to keep this as an "upstream tracking" ticket, as sagenb will still need to be upgraded for this (and whatever else - sagemath/sagenb#267 could use review, for instance).

[jdemeyer]

comment:9

OK, I'll rework the ticket.

[kcrisman]

comment:31

Okay, I think it is stuff like sagemath/sagenb@5ffa976 - it actually happens in other places we import babel (or flask.ext.babel) but those were not doctested in Sage.

Unfortunately, the util/fetch_deps.py in sagenb doesn't exactly make it easy to just apply patches.

[kcrisman]

comment:32

Yes, but the previous Notebook version didn't actually use babel.

Sure it did - e.g. sagemath/sagenb@5511a2a

[kcrisman]

comment:33

Unfortunately, the util/fetch_deps.py in sagenb doesn't exactly make it easy to just apply patches.

Worst case we could put this file in the egg at the very end, but that is pretty hackish.

[kcrisman]

comment:34

By the way, if you or others could test the rest of the upgrade, or at any rate normal functioning of nb, then we could have just this issue to deal with.

[jdemeyer]

comment:35

Yes, I was sloppy. To be more precise: running the doctests with sagenb-0.11.1 didn't import the babel package.

To patch babel: it's probably best to remove babel from sagenb and install it like a normal package (this should really be done for all packages inside sagenb).

[jdemeyer]

comment:36

At least, the babel failure is the only doctest failure.

[kcrisman]

comment:37

To patch babel: it's probably best to remove babel from sagenb and install it like a normal package (this should really be done for all packages inside sagenb).

Yes, but easier said than done. I won't have time for this today, and I'm a little hazy on exactly how all the dependencies for sagenb might get properly imported from Sage itself.

It would also be nice if upstream were responsive at all to their many pull requests (this seems to be the case for several projects pocoo has "taken over").

[jdemeyer]

comment:38

Alternatively, if you just want to fix the doctest failure, you could import the babel stuff inside the method where you need it.

[kcrisman]

comment:39

Hmm, that might be easier.

[kcrisman]

comment:40

Hmm, that might be easier.

Not that I wouldn't be averse to shipping the dependencies separately again! I just feel like any time I have for sagenb is better used for actual bugs, of which there are already plenty.

[kcrisman]

comment:41

Hmm, that might be easier.

Yes, that fixes it - and since there is only that one function, it's not a problem to import it in that one place.

I'll have a fix and new package in a little bit. I hope.

[sagetrac-git]

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

14d3291

Upgrade sagenb to version 0.11.2

[sagetrac-git]

Changed commit from f5c559d to 14d3291

[kcrisman]

Reviewer: Jeroen Demeyer

[kcrisman]

comment:43

Okay, it was a forced push but should be okay now. Same address for the updated spkg. Here is the diff - once this has positive review I'll push it all back upstream.

diff --git a/sagenb/misc/misc.py b/sagenb/misc/misc.py
index 88b3eef..5a647f7 100644
--- a/sagenb/misc/misc.py
+++ b/sagenb/misc/misc.py
@@ -20,8 +20,6 @@ Check that github issue #195 is fixed::
 #############################################################################
 
 from pkg_resources import resource_filename
-from babel import Locale
-from babel.core import UnknownLocaleError
 
 def stub(f):
     def g(*args, **kwds):
@@ -517,6 +515,8 @@ def translations_path():
     return os.path.join(SAGENB_ROOT, 'translations')
 
 def get_languages():
+    from babel import Locale
+    from babel.core import UnknownLocaleError
     langs = []
     dir_names = [lang for lang in os.listdir(translations_path())
                        if lang != 'en_US']

[jdemeyer]

comment:44

All long doctests pass this time.

[kcrisman]

comment:45

And I assume that it is still a gnu tar archive?

Okay, I think anyone can put this to positive review if they don't see any regressions in actual behavior.

[kcrisman]

comment:46

I'm tagging this upstream, so any additional problems will have to be 1) after Christmas and 2) version 0.11.3.

[jdemeyer]

comment:47

The tarball is made correctly, I don't get any warnings when extracting it.

I also checked that the tarball contents matches the git repo. I noticed a few missing files: sagemath/sagenb#327 I consider this a serious issue (especially with the COPYING file missing), but not serious enough to block this ticket.

[jdemeyer]

comment:48

• bug in canvas3d html5 rendering of 3d plots #11275: fixed
• @interact matrix input control ignores label #8738: fixed
• notebook fortran mode does not work with blank first line (or subroutine starting on first line) #8427: fixed
• Fix tar issue in sagenb #17490: fixed

[vbraun]

Changed branch from u/kcrisman/ticket/10057 to 14d3291