Warning: Can't synchronize with repository "(default)" (Unsupported version control system "svn": No module named svn). Look in the Trac log for more information.

Ticket #2499 (closed defect: migrated)

Opened 9 years ago

Last modified 8 years ago

__repr__ methods of Classes in auth.py return Unicode

Reported by: pipoun Owned by: Chris Arndt
Priority: normal Milestone: 2.1
Component: Quickstart Templates Version: 2.1
Severity: minor Keywords:
Cc:

Description

In model/auth.py, the __repr__ methods are returning unicode string. It's not the correct behavior, and I except UnicodeEncodeError? when printing some of the objects.

class Group(DeclarativeBase):
    group_name = Column(Unicode(16), unique=True, nullable=False)
    def __repr__(self):
        return '<Group: name=%s>' % self.group_name

As self.group_name is unicode, __repr__ returns unicode.

Perhaps return repr('<Group: name=%s>' % self.group_name) instead?

This mistake is annoying because auth.py contains the first functional classes that a new TG dev will see (more complete than in model.template). So they will serve as a reference for future classes.

Change History

comment:1 follow-up: ↓ 6 Changed 9 years ago by chrisz

I would just replace the %s by a %r.

comment:2 Changed 9 years ago by chrisz

  • Milestone changed from __unclassified__ to 2.1rc1

comment:4 Changed 9 years ago by mramm

Seems like it could be related to:

 http://bugs.python.org/issue5876

There's nothing inherently wrong with unicode in an repr and it seems to work fine in recent python versions that I have installed here....

comment:5 Changed 9 years ago by mramm

  • Severity changed from normal to minor

comment:6 in reply to: ↑ 1 ; follow-up: ↓ 9 Changed 9 years ago by mramm

  • Status changed from new to closed
  • Resolution set to invalid

Replying to chrisz:

I would just replace the %s by a %r.

But then the result will be allowed to be (and will be) a Unicode string. And by the way the current code uses %r, so that means doing nothing.

I expect the right thing to do here is to not change the template and work towards depricating the old versions of python where implicit calling of repr causes failures when repr results are a unicode string.

The other option would be to throw a .encode('utf8') on the end of the strings here...

comment:7 Changed 9 years ago by cito

  • Status changed from closed to reopened
  • Resolution invalid deleted

I need to dissent:

  • The  Py docs say about __repr__: "The return value must be a string object".
  • repr() is intended to be printed on the console or in logs, and unicode will be often converted to ascii there, resulting in UnicodeErrors.
  • The mentioned  bug is not fixed in the current 2.6 and 2.7 versions and will never be fixed in 2.5, we must live with it.
  • If you replace the %s by a %r the result will be a string.

Please try this if you don't believe me:

class Group:
    group_name = u"La D\xfcsseldorf"
    def __init__(self, format):
        self.format = format
    def __repr__(self):
        return self.format % self.group_name

print repr(Group('%r')) # works
print repr(Group('%s')) # UnicodeEncodeError

comment:8 Changed 9 years ago by pipoun

I second cito. And I don't understand the last comment of Mark

I would just replace the %s by a %r.

But then the result will be allowed to be (and will be) a Unicode string. And by the way the current code uses %r, so that means doing nothing.

No, the result will be a byte string, not an unicode string.

__repr__ has to return a byte string, not an unicode string, this recommendation is completely independent of the python version.

Using string formatting and the conversion type 'r' instead of 's' seems to be a good solution. Means calling repr() instead of str() on the arguments. In our case it's self.group_name
 http://docs.python.org/library/stdtypes.html#string-formatting-operations

class Group(DeclarativeBase):
    group_name = Column(Unicode(16), unique=True, nullable=False)
    def __repr__(self):
        return '<Group: name=%r>' % self.group_name

I have not been able to find the current code in the svn trunk. So if it's already fixed (my ticket was based on a TG 2.0 quickstart), I think we're done here.

My point was that the auth.py file should show a perfect code, because it will serve as a model for newbies.

comment:9 in reply to: ↑ 6 Changed 9 years ago by cito

  • Status changed from reopened to closed
  • Resolution set to fixed

Replying to mramm:

Replying to chrisz: And by the way the current code uses %r, so that means doing nothing.

Oops, right. I had completely forgotten that I  already fixed this 8 months ago. So in fact, we don't need to do anything and I will close this as fixed. The main problem here was that we didn't get 2.1 out so people are still reporting old 2.0 bugs.

Anyway, the fix was necessary and I agree with pipoun in that we should make the quickstart templates as clean and polished as possible since people learn and copy from them.

comment:10 Changed 9 years ago by pipoun

My bad, I was looking at:
http://trac.turbogears.org/browser/projects/tg.devtools/trunk/devtools/templates/turbogears/%2Bpackage%2B/model/auth.py_tmpl

and I should have looked at:
 http://bitbucket.org/turbogears/tgdevtools-dev/src/3009886ac7dd/devtools/templates/turbogears/+package+/model/auth.py_tmpl

Anyway, Mark and Cristoph, you'll have to reach an accord, because I see .encode('utf-8') in the last commit. (with sometimes %r and sometimes %s)

    def __repr__(self):
        return ('<Group: name=%s>' % self.group_name).encode('utf-8')

    def __repr__(self):
        return ('<User: name=%r, email=%r, display=%r>' % (
                self.user_name, self.email_address, self.display_name)).encode('utf-8')

    def __repr__(self):
        return ('<Permission: name=%r>' % self.permission_name).encode('utf-8')

comment:11 Changed 9 years ago by cito

  • Status changed from closed to reopened
  • Resolution fixed deleted

Ah, I hadn't noticed that it was changed again during the last sprint (unfortunately, I could not attend).

As explained above, I think that last change should be reverted. In any case it needs to be handled consistently.

comment:12 Changed 9 years ago by pipoun

From what I can see in the last commit, this has been fixed. Only %r formatting are used, without utf8 encoding.

 http://bitbucket.org/turbogears/tgdevtools-dev/src/0e6e00e5c672/devtools/templates/turbogears/+package+/model/auth.py_tmpl

So, maybe we should close the ticket?

comment:13 Changed 9 years ago by pipoun

Sorry, forget my last comment, I got lost in the changesets.

 http://bitbucket.org/turbogears/tgdevtools-dev/history/devtools/templates/turbogears/+package+/model/auth.py_tmpl?page=4

Does not show the latest commit. I should improve my bitbucket skills.

comment:14 Changed 9 years ago by percious

  • Version changed from 2.0.3 to 2.1
  • Milestone changed from 2.1rc1 to 2.1

comment:15 Changed 8 years ago by pedersen

  • Status changed from reopened to closed
  • Resolution set to migrated
Note: See TracTickets for help on using tickets.