Stop using old_style_globals

[jdemeyer]

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.

Depends on #12446
Depends on #18084

Component: cython

Author: Jeroen Demeyer

Branch/Commit: u/jdemeyer/stop_using_old_style_globals @ 9ccd889

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

[jdemeyer]

Branch: u/jdemeyer/stop_using_old_style_globals

[jdemeyer]

Changed dependencies from #12446 to #12446, #18084

[jdemeyer]

Commit: 265b56c

[jdemeyer]

New commits:

b5b2726	Introduce user_globals
704007e	Make it an error to use user_globals with initialization
265b56c	Use user_globals instead of Cython's old_style_globals

[sagetrac-git]

Changed commit from 265b56c to e4fc3ea

[sagetrac-git]

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

0056d0a	Do not use var() in the Sage library
f680be3	Use SR.var()
b0a4317	Merge commit 'f680be3a9f7da9ca3957a9709997a30917bba687' into HEAD
e4fc3ea	Use user_globals instead of Cython's old_style_globals

[sagetrac-git]

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

[sagetrac-git]

Changed commit from e4fc3ea to c98b7d8

[sagetrac-git]

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

[sagetrac-git]

Changed commit from c98b7d8 to 9ccd889

[nbruin]

comment:7

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:
[#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.

[jdemeyer]

comment:8

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.

[nbruin]

comment:9

Replying to @jdemeyer:

So instead of calling initialize_globals() at global scope, you define inject() 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 *.

[jdemeyer]

comment:10

Replying to @nbruin:

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

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?

[nbruin]

comment:11

Replying to @jdemeyer:

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?

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

[jdemeyer]

comment:12

Replying to @nbruin:

Replying to @jdemeyer:

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?

Internally it should be doing the import or at least the equivalent of it.

That was exactly my question: how?

[nbruin]

comment:13

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.

[jdemeyer]

comment:14

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.

[jdemeyer]

comment:15

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

[jdemeyer]

comment:16

Replying to @nbruin:

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.

Are you implying with this sentence that #12446 isn't "entirely Python"?

[nbruin]

comment:17

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.

[embray]

comment:18

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

[nbruin]

comment:19

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.

[jdemeyer]

comment:20

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