inject_shorthands broken since 6.2beta8

[darijgr]

sage: NSym = NonCommutativeSymmetricFunctions(QQ)
sage: NSym.inject_shorthands()
Injecting S as shorthand for Non-Commutative Symmetric Functions over the Rational Field in the Complete basis
---------------------------------------------------------------------------
KeyError                                  Traceback (most recent call last)
<ipython-input-2-458277a04544> in <module>()
----> 1 NSym.inject_shorthands()

/home/darij/gitsage6.2/local/lib/python2.7/site-packages/sage/categories/sets_cat.pyc in inject_shorthands(self, verbose)
   1318                     if verbose:
   1319                         print 'Injecting %s as shorthand for %s'%(shorthand, realization)
-> 1320                     inject_variable(shorthand, realization)
   1321 
   1322             def realizations(self):

/home/darij/gitsage6.2/local/lib/python2.7/site-packages/sage/misc/misc.pyc in inject_variable(name, value)
   2356     # inject_variable is called not only from the interpreter, but
   2357     # also from functions in various modules.
-> 2358     G = get_main_globals()
   2359     if name in G:
   2360         warn("redefining global value `%s`"%name, RuntimeWarning, stacklevel = 2)

/home/darij/gitsage6.2/local/lib/python2.7/site-packages/sage/misc/misc.pyc in get_main_globals()
   2312     while True:
   2313         G = sys._getframe(depth).f_globals
-> 2314         if G["__name__"] == "__main__" and G["__package__"] is None:
   2315             break
   2316         depth += 1

KeyError: '__package__'

Same for Sym and QSym.

The get_main_globals method hasn't been changed since 2011, so I suspect something else is at fault. The attached branch gets rid of the bug, but I don't understand the code at hand and so it is not really merge-ready. Needs_review is to increase visibility.

CC: @nthiery @vbraun

Component: misc

Keywords: globals, inject_shorthands, regression

Author: Darij Grinberg

Branch/Commit: 648da0d

Reviewer: Travis Scrimshaw

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

[darijgr]

comment:2

Oh, and apparently importing any(!) sage file beforehand keeps the bug from happening. Here is an example that does not throw errors:

sage: import sage.combinat.permutation
sage: NSym = NonCommutativeSymmetricFunctions(QQ)
sage: NSym.inject_shorthands()

[sagetrac-git]

Changed commit from 67855fc to 6e0f42f

[sagetrac-git]

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

6e0f42f

Added a doctest.

[tscrim]

comment:4

Just doing

sage: sage.misc.misc.inject_variable('a', 0)

(before an import) is enough to cause the error. If I had to make a WAG, I'd say it's the IPython upgrade. I've added this as a doctest, so if you're happy this (and I'm okay with fixing the symptoms), positive review.

[darijgr]

comment:5

Unfortunately this doctest works even without the patch :(

And moving it to the very front of the file doesn't help either...

[darijgr]

comment:6

Oops, sorry for removing you from cc, Volker (I really should report this as a trac bug -- I started writing my post before you made yours -- but I'm too lazy).

[tscrim]

comment:7

Hmm...there must be something going on within the doctesting framework that prevents this from begin executed before any imports. We can leave it in there as a human warning, but honestly noone is really going to look at that test (much less run it manually)...

[darijgr]

comment:8

BTW: when I do sys._getframe(0).f_globals, I get the following cruft:

/home/darij/gitsage6.2/local/lib/python2.7/site-packages/IPython/core/interactiveshell.py:2834: DeprecationWarning: this is being removed from the global namespace. Use crystals.elementary.<tab> instead
See http://trac.sagemath.org/15882 for details.
  exec code_obj in self.user_global_ns, self.user_ns
/home/darij/gitsage6.2/local/lib/python2.7/site-packages/IPython/core/interactiveshell.py:2834: DeprecationWarning: this is being removed from the global namespace. Use crystals.<tab> instead
See http://trac.sagemath.org/15882 for details.
  exec code_obj in self.user_global_ns, self.user_ns
/home/darij/gitsage6.2/local/lib/python2.7/site-packages/IPython/core/interactiveshell.py:2834: DeprecationWarning: this is being removed from the global namespace. Use crystals.AlcovePath instead
See http://trac.sagemath.org/15882 for details.
  exec code_obj in self.user_global_ns, self.user_ns
/home/darij/gitsage6.2/local/lib/python2.7/site-packages/IPython/core/interactiveshell.py:2834: DeprecationWarning: this is being removed from the global namespace. Use crystals.FastRankTwo instead
See http://trac.sagemath.org/15882 for details.
  exec code_obj in self.user_global_ns, self.user_ns
/home/darij/gitsage6.2/local/lib/python2.7/site-packages/IPython/core/interactiveshell.py:2834: DeprecationWarning: this is being removed from the global namespace. Use crystals.infinity.Tableaux instead
See http://trac.sagemath.org/15882 for details.
  exec code_obj in self.user_global_ns, self.user_ns
/home/darij/gitsage6.2/local/lib/python2.7/site-packages/IPython/core/interactiveshell.py:2834: DeprecationWarning: this is being removed from the global namespace. Use crystals.TensorProductOfKirillovReshetikhinTableaux instead
See http://trac.sagemath.org/15882 for details.
  exec code_obj in self.user_global_ns, self.user_ns
/home/darij/gitsage6.2/local/lib/python2.7/site-packages/IPython/core/interactiveshell.py:2834: DeprecationWarning: this is being removed from the global namespace. Use crystals.KyotoPathModel instead
See http://trac.sagemath.org/15882 for details.
  exec code_obj in self.user_global_ns, self.user_ns
/home/darij/gitsage6.2/local/lib/python2.7/site-packages/IPython/core/interactiveshell.py:2834: DeprecationWarning: this is being removed from the global namespace. Use crystals.Letters instead
See http://trac.sagemath.org/15882 for details.
  exec code_obj in self.user_global_ns, self.user_ns
/home/darij/gitsage6.2/local/lib/python2.7/site-packages/IPython/core/interactiveshell.py:2834: DeprecationWarning: this is being removed from the global namespace. Use crystals.DirectSum instead
See http://trac.sagemath.org/15882 for details.
  exec code_obj in self.user_global_ns, self.user_ns
/home/darij/gitsage6.2/local/lib/python2.7/site-packages/IPython/core/interactiveshell.py:2834: DeprecationWarning: this is being removed from the global namespace. Use crystals.KirillovReshetikhin with model='KR' instead
See http://trac.sagemath.org/15882 for details.
  exec code_obj in self.user_global_ns, self.user_ns
/home/darij/gitsage6.2/local/lib/python2.7/site-packages/IPython/core/interactiveshell.py:2834: DeprecationWarning: this is being removed from the global namespace. Use crystals.KirillovResetikhin instead
See http://trac.sagemath.org/15882 for details.
  exec code_obj in self.user_global_ns, self.user_ns
/home/darij/gitsage6.2/local/lib/python2.7/site-packages/IPython/core/interactiveshell.py:2834: DeprecationWarning: this is being removed from the global namespace. Use crystals.HighestWeight instead
See http://trac.sagemath.org/15882 for details.
  exec code_obj in self.user_global_ns, self.user_ns
/home/darij/gitsage6.2/local/lib/python2.7/site-packages/Babel-1.3-py2.7.egg/babel/core.py:14: ImportWarning: Not importing directory '/home/darij/gitsage6.2/local/lib/python2.7/site-packages/Babel-1.3-py2.7.egg/babel/localedata': missing __init__.py
  from babel import localedata

And this is just the error output. Here is the actual dictionary: https://www.dropbox.com/s/6hzuqm6jf9jo9jt/outtext.txt

I can't believe this all needs to be in the globals?

[tscrim]

comment:9

Those deprecations are from #15882 (ex. CrystalOfTableaux) where I used #14275 to (lazily) import them with a deprecation message and there was no good alternative that I could find (because I didn't want to write some 20 functions where all they did was deprecate something with a [semi]specific message). Moreover, I'd be surprised if they had any bearing on this issue.

[darijgr]

comment:10

Yeah, I now see it's pretty much orthogonal to what we're doing here. Good to know we've got a delsarte_bound_additive_hamming_space function in our global namespace, though :)

[darijgr]

comment:11

From sage/doctest/forker.py:

                # Make the right set of globals available to doctests
                if basename.startswith("sagenb."):
                    import sage.all_notebook as sage_all
                else:
                    import sage.all_cmdline as sage_all
                sage_namespace = RecordingDict(sage_all.__dict__)
                sage_namespace['__name__'] = '__main__'
                sage_namespace['__package__'] = None
                doctests, extras = self.source.create_doctests(sage_namespace)
                timer = Timer().start()

I'm wondering if we should just axe the line with the __package__ here?

[darijgr]

comment:12

OK, axing doesn't work, since this line is there to undo the (apparently Sphinx-specific) setting of __package__ to sage. Let me try del...

[darijgr]

comment:13

EDIT: ignore this post.

[sagetrac-git]

Changed commit from 6e0f42f to cebac21

[sagetrac-git]

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

cebac21

fixing the broken inject_shorthands methods, and modifying doctest framework so that this fix can be tested for

[darijgr]

comment:15

OK, forget about the second branch; I've just hijacked the main branch with what I think is a full solution. Warning: forced push!

[sagetrac-git]

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

0b8ed45

same change in toric varieties code

[sagetrac-git]

Changed commit from cebac21 to 0b8ed45

[tscrim]

Reviewer: Travis Scrimshaw

[tscrim]

Author: Darij Grinberg

[tscrim]

comment:17

LGTM.

[darijgr]

comment:18

Thank you, Travis!!

blush I guess I really should start reviewing...

[vbraun]

comment:19

• Can we not leave commented-out lines lying around?

• IMHO the check should just be

  if G.get("__name__", None) == "__main__":
  

  and not look at __package__. The topmost '__main__' frame is our guy, duh. Also this won't trip over similar issues if some frame doesn't have __name__.

• The new doctest is just like doctests that we already have, it doesn't add anything new.

[sagetrac-git]

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

94a1e8b

fixes suggested by Volker

[sagetrac-git]

Changed commit from 0b8ed45 to 94a1e8b

[sagetrac-git]

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

648da0d

oops

[sagetrac-git]

Changed commit from 94a1e8b to 648da0d

[darijgr]

comment:22

ad 1: True; that was a habit from pre-git times. Removed.

ad 2: I have changed it now but I'm not 100% sure (what happens if we are doctesting a package?). I don't have time to run all the doctests now...

ad 3: I can't follow this. It is import-less and it stands at the beginning of the file. At least the first of these properties is crucial!

[vbraun]

Changed branch from public/combinat/inject_shorthands to 648da0d