#16181 closed defect (fixed)

inject_shorthands broken since 6.2beta8

Reported by: darij Owned by:
Priority: critical Milestone: sage-6.2
Component: misc Keywords: globals, inject_shorthands, regression,
Cc: nthiery, vbraun Merged in:
Authors: Darij Grinberg Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 648da0d (Commits) Commit: 648da0d3ee5d408f210e3f36a66d6c4f5e1bffb6
Dependencies: Stopgaps:

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.

Change History (24)

comment:1 Changed 15 months ago by darij

  • Status changed from new to needs_review

comment:2 Changed 15 months 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 15 months ago by git

  • Commit changed from 67855fc66eed143172a9065928e52241d97edefe to 6e0f42f336ad394f05040e379252c18c5785967a

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

6e0f42fAdded a doctest.

comment:4 Changed 15 months ago by tscrim

  • Cc vbraun added

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 15 months ago by tscrim (previous) (diff)

comment:5 Changed 15 months 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 15 months ago by darij

  • Cc vbraun added

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 15 months 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 15 months 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 15 months 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 15 months 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 15 months 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 15 months 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 15 months ago by darij

EDIT: ignore this post.

Last edited 15 months ago by darij (previous) (diff)

comment:14 Changed 15 months ago by git

  • Commit changed from 6e0f42f336ad394f05040e379252c18c5785967a to cebac21b1ba3372d561760260d24442a40e75ada

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

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

comment:15 Changed 15 months 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 15 months ago by darij (previous) (diff)

comment:16 Changed 15 months ago by git

  • Commit changed from cebac21b1ba3372d561760260d24442a40e75ada to 0b8ed458f20f651b1087da7eae3d182c5801e540

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

0b8ed45same change in toric varieties code

comment:17 Changed 15 months 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 15 months ago by darij

Thank you, Travis!!

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

comment:19 Changed 15 months 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 15 months ago by git

  • Commit changed from 0b8ed458f20f651b1087da7eae3d182c5801e540 to 94a1e8bbb2d67f5474d17aec16bbe83c2884167c

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

94a1e8bfixes suggested by Volker

comment:21 Changed 15 months ago by git

  • Commit changed from 94a1e8bbb2d67f5474d17aec16bbe83c2884167c to 648da0d3ee5d408f210e3f36a66d6c4f5e1bffb6

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

648da0doops

comment:22 Changed 15 months 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 15 months ago by darij (previous) (diff)

comment:23 Changed 15 months ago by darij

  • Status changed from needs_info to needs_review

comment:24 Changed 15 months 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.