Opened 6 years ago

Last modified 21 months ago

#18083 new enhancement

Stop using old_style_globals

Reported by: jdemeyer Owned by:
Priority: major Milestone: sage-6.6
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:

Status badges

Description (last modified by 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.

Change History (20)

comment:1 Changed 6 years ago by jdemeyer

  • Branch set to u/jdemeyer/stop_using_old_style_globals

comment:2 Changed 6 years ago by jdemeyer

  • Commit set to 265b56c8f0256c3a31aecf7f03bdc14c095b8185
  • Dependencies changed from #12446 to #12446, #18084

New commits:

b5b2726Introduce user_globals
704007eMake it an error to use user_globals with initialization
265b56cUse user_globals instead of Cython's old_style_globals

comment:3 Changed 6 years ago by git

  • Commit changed from 265b56c8f0256c3a31aecf7f03bdc14c095b8185 to e4fc3ea13a2c7846cf012ad00acba22953ac05f4

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

0056d0aDo not use var() in the Sage library
f680be3Use SR.var()
b0a4317Merge commit 'f680be3a9f7da9ca3957a9709997a30917bba687' into HEAD
e4fc3eaUse user_globals instead of Cython's old_style_globals

comment:4 Changed 6 years ago by git

  • Commit changed from e4fc3ea13a2c7846cf012ad00acba22953ac05f4 to c98b7d8aff011348a86729eac651227d60b6eb19

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

c98b7d8Use user_globals instead of Cython's old_style_globals

comment:5 Changed 6 years ago by jdemeyer

  • Description modified (diff)

comment:6 Changed 6 years ago by git

  • Commit changed from c98b7d8aff011348a86729eac651227d60b6eb19 to 9ccd889321d311903d4ab26449e6dd8154879aee

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

9ccd889Use user_globals instead of Cython's old_style_globals

comment:7 follow-up: Changed 6 years ago by nbruin

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 in reply to: ↑ 7 ; follow-up: Changed 6 years ago by jdemeyer

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 in reply to: ↑ 8 ; follow-up: Changed 6 years ago by nbruin

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

comment:10 in reply to: ↑ 9 ; follow-up: Changed 6 years ago by jdemeyer

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?

comment:11 in reply to: ↑ 10 ; follow-up: Changed 6 years ago by 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?

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 in reply to: ↑ 11 ; follow-up: Changed 6 years ago by jdemeyer

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?

comment:13 in reply to: ↑ 12 ; follow-ups: Changed 6 years ago by 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.

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 in reply to: ↑ 13 Changed 6 years ago by jdemeyer

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 in reply to: ↑ 13 Changed 6 years ago by jdemeyer

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 in reply to: ↑ 13 ; follow-up: Changed 6 years ago by jdemeyer

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"?

comment:17 in reply to: ↑ 16 Changed 6 years ago by nbruin

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 3 years ago by embray

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: Changed 21 months ago by nbruin

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 in reply to: ↑ 19 Changed 21 months ago by jdemeyer

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

Note: See TracTickets for help on using tickets.