Opened 8 years ago

Closed 8 years ago

# inject_shorthands broken since 6.2beta8

Reported by: Owned by: darij critical sage-6.2 misc globals, inject_shorthands, regression, nthiery, vbraun Darij Grinberg Travis Scrimshaw N/A 648da0d 648da0d3ee5d408f210e3f36a66d6c4f5e1bffb6

### Description

```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.

### comment:1 Changed 8 years ago by darij

• Status changed from new to needs_review

### comment:2 Changed 8 years ago by darij

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()
```

### comment:3 Changed 8 years ago by git

• Commit changed from 67855fc66eed143172a9065928e52241d97edefe to 6e0f42f336ad394f05040e379252c18c5785967a

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

 ​6e0f42f `Added a doctest.`

### comment:4 Changed 8 years ago by tscrim

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.

Last edited 8 years ago by tscrim (previous) (diff)

### comment:5 Changed 8 years ago by darij

• Cc vbraun removed

Unfortunately this doctest works even without the patch :(

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

### comment:6 Changed 8 years ago by darij

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).

### comment:7 Changed 8 years ago by tscrim

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)...

### comment:8 Changed 8 years ago by darij

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?

### comment:9 Changed 8 years ago by tscrim

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.

### comment:10 Changed 8 years ago by darij

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 :)

### comment:11 Changed 8 years ago by darij

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?

### comment:12 Changed 8 years ago by darij

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`...

### comment:13 Changed 8 years ago by darij

EDIT: ignore this post.

Last edited 8 years ago by darij (previous) (diff)

### comment:14 Changed 8 years ago by 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`

### comment:15 Changed 8 years ago by darij

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

Last edited 8 years ago by darij (previous) (diff)

### comment:16 Changed 8 years ago by git

• Commit changed from cebac21b1ba3372d561760260d24442a40e75ada to 0b8ed458f20f651b1087da7eae3d182c5801e540

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

 ​0b8ed45 `same change in toric varieties code`

### comment:17 Changed 8 years ago by tscrim

• Authors set to Darij Grinberg
• Reviewers set to Travis Scrimshaw
• Status changed from needs_review to positive_review

LGTM.

### comment:18 Changed 8 years ago by darij

Thank you, Travis!!

*blush* I guess I really should start reviewing...

### comment:19 Changed 8 years ago by vbraun

• Status changed from positive_review to needs_info
• 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.

### comment:20 Changed 8 years ago by git

• Commit changed from 0b8ed458f20f651b1087da7eae3d182c5801e540 to 94a1e8bbb2d67f5474d17aec16bbe83c2884167c

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

 ​94a1e8b `fixes suggested by Volker`

### comment:21 Changed 8 years ago by git

• Commit changed from 94a1e8bbb2d67f5474d17aec16bbe83c2884167c to 648da0d3ee5d408f210e3f36a66d6c4f5e1bffb6

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

 ​648da0d `oops`

### comment:22 Changed 8 years ago by darij

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!

Last edited 8 years ago by darij (previous) (diff)

### comment:23 Changed 8 years ago by darij

• Status changed from needs_info to needs_review

### comment:24 Changed 8 years ago by vbraun

• Branch changed from public/combinat/inject_shorthands to 648da0d3ee5d408f210e3f36a66d6c4f5e1bffb6
• Resolution set to fixed
• Status changed from needs_review to closed
Note: See TracTickets for help on using tickets.