Opened 8 years ago
Last modified 5 weeks ago
#18083 new enhancement
Stop using old_style_globals
Reported by: | jdemeyer | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | |
Component: | cython | Keywords: | |
Cc: | Merged in: | ||
Authors: | Jeroen Demeyer | Reviewers: | |
Report Upstream: | N/A | Work issues: | |
Branch: | u/jdemeyer/stop_using_old_style_globals (Commits, GitHub, GitLab) | Commit: | 9ccd889321d311903d4ab26449e6dd8154879aee |
Dependencies: | #12446, #18084 | Stopgaps: |
Description (last modified by )
Instead of using Cython's old_style_globals
option, we use the new user_globals
from #12446 to access globals()
.
In order to support
sage: sage_eval( ("f(x)=x^2", "f(1)") )
we make a small change to the preparser: we ensure that f(x) = ...
makes x
available both as global and as local.
Change History (21)
comment:1 Changed 8 years ago by
Branch: | → u/jdemeyer/stop_using_old_style_globals |
---|
comment:2 Changed 8 years ago by
Commit: | → 265b56c8f0256c3a31aecf7f03bdc14c095b8185 |
---|---|
Dependencies: | #12446 → #12446, #18084 |
comment:3 Changed 8 years ago by
Commit: | 265b56c8f0256c3a31aecf7f03bdc14c095b8185 → e4fc3ea13a2c7846cf012ad00acba22953ac05f4 |
---|
comment:4 Changed 8 years ago by
Commit: | e4fc3ea13a2c7846cf012ad00acba22953ac05f4 → c98b7d8aff011348a86729eac651227d60b6eb19 |
---|
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
c98b7d8 | Use user_globals instead of Cython's old_style_globals
|
comment:5 Changed 8 years ago by
Description: | modified (diff) |
---|
comment:6 Changed 8 years ago by
Commit: | c98b7d8aff011348a86729eac651227d60b6eb19 → 9ccd889321d311903d4ab26449e6dd8154879aee |
---|
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
9ccd889 | Use user_globals instead of Cython's old_style_globals
|
comment:7 follow-up: 8 Changed 8 years ago by
Just an idea:
Logically, we can't really ask an object to inject itself into "the" globals namespace, because the object doesn't really have access to what that namespace is in the relevant situation.
Instead what one can do from the REPL is to bind an object to its print name, which is what injection usually does. Then one actually doesn't need any special machinery on objects. See: http://trac.sagemath.org/ticket/17958#comment:54
In principle then one can spell:
sage: x = SR.var('x') sage: inject(x)
where inject
could be defined in the "global scope" (by injecting its definition into the start-up, like .sage
):
def inject(a): globals[str(a)]=a
except that we'd want to do some validation on str(a)
before we do that.
It can also be shortened to inject(SR.var('x'))
to avoid repetition.
Of course to type this is very inconvenient, so we'd need some further wrappers to make the syntax more palatable (the %var
directive seems like a good step), but the simplicity of the implementation and the flexibility of the construction leads me to believe that this is the right way of providing injection capability.
comment:8 follow-up: 9 Changed 8 years ago by
Replying to nbruin:
where
inject
could be defined in the "global scope" (by injecting its definition into the start-up, like.sage
):def inject(a): globals[str(a)]=a
So instead of calling initialize_globals()
at global scope, you define inject()
at global scope. That doesn't seem more simple or flexible.
And I don't understand how you would implement cython()
using your approach.
comment:9 follow-up: 10 Changed 8 years ago by
Replying to jdemeyer:
So instead of calling
initialize_globals()
at global scope, you defineinject()
at global scope. That doesn't seem more simple or flexible.
No, it doesn't make much practical difference. You're basically storing the dictionary on inject.func_globals
instead of a custom store. I think it's useful to see that there's a "natural" way of expressing the concept in pure python. What the actual implementation underneath is, is another matter. I think it's worthwhile to give some consideration to using an implementation that is "natural" to python. The "natural" place for an inject to live is in the user scope, not as a method on various objects.
And I don't understand how you would implement
cython()
using your approach.
That's a matter of import, isn't it? Compiling cython normally gives a module tmp_cython_module
and with cython
you basically do a from tmp_cython_module import *
.
comment:10 follow-up: 11 Changed 8 years ago by
Replying to nbruin:
That's a matter of import, isn't it? Compiling cython normally gives a module
tmp_cython_module
and withcython
you basically do afrom tmp_cython_module import *
.
So the one-liner cython("foo")
is going to change to looking at the output of cython()
plus a manual import
, probably with some sys.path
manipulation thrown in?
comment:11 follow-up: 12 Changed 8 years ago by
Replying to jdemeyer:
So the one-liner
cython("foo")
is going to change to looking at the output ofcython()
plus a manualimport
, probably with somesys.path
manipulation thrown in?
No, I wasn't proposing to change the user interface of cython
at all. Internally it should be doing the import
or at least the equivalent of it.
My observation is just that in the architecture of python, it is more natural to ask the REPL to inject a binding than to ask an object to inject itself into the global scope of the REPL. Hence, we might end up with simpler, easier to maintain code if we follow that design and use as a fundamental building block a service provided by the REPL.
For things like var
and cython
the interface is already so nice (modulo some other issues) that we should probably not change the spelling. But for
QQ['x','y'].inject_generators()
it might be nicer (and easier to implement!) to spell it as
inject(Q['x','y'].gens())
comment:12 follow-up: 13 Changed 8 years ago by
Replying to nbruin:
Replying to jdemeyer:
So the one-liner
cython("foo")
is going to change to looking at the output ofcython()
plus a manualimport
, probably with somesys.path
manipulation thrown in?Internally it should be doing the
import
or at least the equivalent of it.
That was exactly my question: how?
comment:13 follow-ups: 14 15 16 Changed 8 years ago by
Replying to jdemeyer:
That was exactly my question: how?
Logically the (very thin!) wrapper cython
might be injected into the same global scope as inject is written in.
Alternatively, once "inject" has been defined, we could inject it:
sage.misc.cython.inject = inject #we don't need cython_c anymore
ready for use there. Or one could access the dictionary there via inject.func_globals (which wouldn't be so clean).
Admittedly, this is all not so much cleaner than what happens now. The central "cleaner" principle is that there is a way of capturing a "globals" dictionary: define a function there. That should be a portable principle (accessing func_globals directly is probably not strictly portable).
I'm not suggesting that we should absolutely implement it like this, but I think it might be useful to design the underlying code in a way that it *might* have been implemented in this way, simply to stay close to "the python way" of doing things. I don't think the python way is sacred, but since we're basing things on python, we should have good reasons every time we do deviate from it. And I'm not so sure that for injection we do have such a good reason.
One "python way" would be to define in, say, sage.misc.inject
def inject(...): raise RuntimeError("injecting code in an environment where injection isn't properly defined")
and then have the different interfaces/REPLs monkey patch an appropriate "inject" into sage.misc.inject.inject.
I'm not so sure how clean monkey patching is and how robust this is against from sage.misc.inject import inject
, but at least it's an entirely python solution.
comment:14 Changed 8 years ago by
Replying to nbruin:
Replying to jdemeyer:
That was exactly my question: how?
Logically the (very thin!) wrapper
cython
might be injected into the same global scope as inject is written in.
Sorry, I don't understand this sentence.
Alternatively, once "inject" has been defined, we could inject it:
sage.misc.cython.inject = inject #we don't need cython_c anymore
So everybody's Sage code should start with the boilerplate
sage.calculus.var.inject = inject sage.calculus.function.inject = inject sage.calculus.desolver.inject = inject sage.misc.cython.inject = inject sage.misc.fortran.inject = inject
Not an elegant solution. Of course you could say "let's define inject
in just one place" and call it from there, which is essentially my proposal.
comment:15 Changed 8 years ago by
Replying to nbruin:
to stay close to "the python way" of doing things.
The closest Python statement to what we're doing is import
and I don't know how one would implement import
in pure Python. So I would say there is really no Python equivalent...
comment:16 follow-up: 17 Changed 8 years ago by
comment:17 Changed 8 years ago by
Replying to jdemeyer:
Are you implying with this sentence that #12446 isn't "entirely Python"?
No, it is. I originally thought that "capturing" the correct globals()
by defining a function in the relevant scope was cleaner than calling sage.repl.user_globals.initialize_globals(...)
, but that's not really the case.
In fact, one way a REPL could initialize globals is by injecting sage.repl.user_globals.initialize_globals(globals())
into its "user input stream" at the start, which should work across REPLs. So I think your implementation is fine.
I do think that sage.repl.user_globals
is a dirty module, because its super-global side effects (that's its point--it's scribbling in a scope it shouldn't really have access to by normal python semantics) and hence should be used in as few places as possible in the library. So I'm still of the opinion that we'd be better off deprecating the .inject_generators()
methods on various parents, in favour of a global scope function inject(QQ['x,y'].gens)
.
The function inject
would then normally not be available in library modules: it would need an explicit import.
comment:18 Changed 5 years ago by
Just discovered the existence of old_style_globals
after some headscratching...
Any thoughts about the status of this ticket? In the meantime I'll add old_style_globals
to this module that needs it...
comment:19 follow-up: 20 Changed 3 years ago by
Just ran into this old ticket myself, looking at how we can work around sage/calculus/var.pyx
:
G = globals() # this is the reason the code must be in Cython.
There is a way to write such code in python directly. Something like:
G = sys._getframe(1).f_globals
should do the trick in most cases (if the caller's "globals" is a read-only dict itself, as a cython module "globals" would be, then one shouldn't be injecting variables in it anyway). I suspect that with old_style_globals=False
the same line should work in cython code as well.
comment:20 Changed 3 years ago by
Cython doesn't generate a stack frame, so sys._getframe(1).f_globals
is wrong, it should be sys._getframe(0).f_globals
. But that's a complicated way to write the builtin globals()
, so we might as well use that.
Indeed, using old_style_globals
is basically equivalent to doing
from builtins import globals
This makes a difference because this overrides the Cython-specific globals()
.
comment:21 Changed 5 weeks ago by
Milestone: | sage-6.6 |
---|
New commits:
Introduce user_globals
Make it an error to use user_globals with initialization
Use user_globals instead of Cython's old_style_globals