Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Behavior of Builtins #83

Closed
sqlalchemy-bot opened this issue May 22, 2008 · 2 comments
Closed

Behavior of Builtins #83

sqlalchemy-bot opened this issue May 22, 2008 · 2 comments

Comments

@sqlalchemy-bot
Copy link

@sqlalchemy-bot sqlalchemy-bot commented May 22, 2008

Migrated issue, originally created by Anonymous

Currently there is some weird behavior regarding identifiers named after builtins. I notived it when using a Mako template written for an application running on Python 2.4 with Python 2.5 which introduced "all" as builtin. The template used that as identifier and after the upgrade to 2.5 the template broke.

Turns out that currently SQLAlchemy doesn't pull identifiers named after builtins from the context. This appears to be expected behavior as there is a unittest for that which apparently doesn't fail or at least not in that situation.

Currently the template "${id}" generates this code:

from mako import runtime, filters, cache
UNDEFINED = runtime.UNDEFINED
_magic_number = 2
_modified_time = 1211488200.114598
_template_filename=None
_template_uri='memory:0x7077d0'
_template_cache=cache.Cache(__name__, _modified_time)
_source_encoding=None
_exports = []


def render_body(context,**pageargs):
    context.caller_stack._push_frame()
    try:
        __M_locals = dict(pageargs=pageargs)
        __M_writer = context.writer()
        # SOURCE LINE 1
        __M_writer(unicode(id))
        return ''
    finally:
        context.caller_stack._pop_frame()

I suggest generating this code instead:

import __builtin__
from mako import runtime, filters, cache
UNDEFINED = runtime.UNDEFINED
_magic_number = 2
_modified_time = 1211488200.114598
_template_filename=None
_template_uri='memory:0x7077d0'
_template_cache=cache.Cache(__name__, _modified_time)
_source_encoding=None
_exports = []


def render_body(context,**pageargs):
    context.caller_stack._push_frame()
    try:
        __M_locals = dict(pageargs=pageargs)
        id = context.get('id', __builtin__.id)
        __M_writer = context.writer()
        # SOURCE LINE 1
        __M_writer(unicode(id))
        return ''
    finally:
        context.caller_stack._pop_frame()

While the lookup of the builtin will be slowed down a bit the actual usage of it is faster because it's a local variable then which isn't looked up in a dict.

I have a possible patch here: http://paste.pocoo.org/show/52822/

However that patch causes the current test for the old behavior to break. If the old behavior is really the desired one that should be documented at least.

@sqlalchemy-bot
Copy link
Author

@sqlalchemy-bot sqlalchemy-bot commented May 24, 2008

Michael Bayer (@zzzeek) wrote:

237e918

@sqlalchemy-bot
Copy link
Author

@sqlalchemy-bot sqlalchemy-bot commented May 24, 2008

Changes by Michael Bayer (@zzzeek):

  • changed status to closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant