Problems because of changing the reference class

53 views
Skip to first unread message

Alan Etkin

unread,
Jun 22, 2013, 11:32:50 AM6/22/13
to web2py-d...@googlegroups.com
I agree that users "traditionally" expect to get an int id from Row objects and that is breaking apps which rely in raw data returned to clients.

https://groups.google.com/d/msg/web2py/4t2N4aeY-Jk/JoSJZ6X00BEJ

Perhaps the Row class could be modified to cast the id output to int if it's lenght does'nt reach the system max size.

Niphlod

unread,
Jun 22, 2013, 2:45:50 PM6/22/13
to web2py-d...@googlegroups.com
I'm -1 on this one....users shouldn't rely on the python default serialization format if they interact with js or html, it's just asking for problems. Simplejson by itself does the job pretty well, and should be encouraged.

One single hiccup shouldn't need web2py to add a check for every id and reference field out there.

Anthony

unread,
Jun 22, 2013, 4:10:52 PM6/22/13
to web2py-d...@googlegroups.com
One single hiccup shouldn't need web2py to add a check for every id and reference field out there.

Actually, it is all integer fields, not just id's and references.

Anthony

Anthony

unread,
Jun 22, 2013, 4:25:18 PM6/22/13
to web2py-d...@googlegroups.com
What about something like this:

class w2p_long(long):
   
def __repr__(self):
       
return self.__str__()

Too confusing?

Anthony

Alan Etkin

unread,
Jun 22, 2013, 4:46:38 PM6/22/13
to
What about something like this:

class w2p_long(long):
   
def __repr__(self):
       
return self.__str__()

Too confusing?

from a user standpoint it is a bit confusing (but transparent since it is not supposed to be part of the api). And also I guess it adds the quotes issue you mentioned before.

Why is this about all integers? The change was only in gluon.dal class Reference. I would prefer the conditional cast to int when generating the Rows object but only if this happens to become a real backwards compatibility problem. I'm not sure this is the case.

EDIT: nevermind the quotes issue. both __str__ and __repr__ are supposed to return strings.

Alan Etkin

unread,
Jun 22, 2013, 6:41:54 PM6/22/13
to web2py-d...@googlegroups.com
BTW: please note (Massimo has pointed out this before) that such way of returning raw reference values would in any case lead to the L appearing when the output passes the max limit even if we revert the Reference class declaration.

Anthony

unread,
Jun 22, 2013, 7:46:19 PM6/22/13
to
What about something like this:

class w2p_long(long):
   
def __repr__(self):
       
return self.__str__()

Too confusing?

from a user standpoint it is a bit confusing (but transparent since it is not supposed to be part of the api).

Yes, it's just an internal class, so that shouldn't necessarily be confusing. I was just wondering if it would be confusing to not see the appended "L" any more when calling the __repr__ method.
 
Why is this about all integers? The change was only in gluon.dal class Reference.

Appears to affect all integer fields:

>>> db.define_table('mytable', Field('myint', 'integer'))
>>> from gluon.contrib import populate
>>> populate.populate(db.mytable, 10)
>>> [r.myint for r in db(db.mytable).select()]
[55L, 19L, 607L, 724L, 194L, 330L, 141L, 229L, 978L, 464L]
 
I would prefer the conditional cast to int when generating the Rows object but only if this happens to become a real backwards compatibility problem. I'm not sure this is the case.

The special class preserves the longs but just changes the serialization to be the same as ints. Not sure it's worth it, but could be a simple solution.

Anthony

Massimo DiPierro

unread,
Jun 23, 2013, 12:47:01 AM6/23/13
to web2py-d...@googlegroups.com
too slow I think.

--
-- mail from:GoogleGroups "web2py-developers" mailing list
make speech: web2py-d...@googlegroups.com
unsubscribe: web2py-develop...@googlegroups.com
details : http://groups.google.com/group/web2py-developers
the project: http://code.google.com/p/web2py/
official : http://www.web2py.com/
---
You received this message because you are subscribed to the Google Groups "web2py-developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to web2py-develop...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
 
 

Anthony

unread,
Jun 23, 2013, 9:19:06 AM6/23/13
to web2py-d...@googlegroups.com
I didn't realize that. What makes it slow?

Massimo DiPierro

unread,
Jun 23, 2013, 9:43:47 AM6/23/13
to web2py-d...@googlegroups.com
I timed this:

for k in xrange(10000000):
    a = 100000000000000
    b = a+a

#1.8sec
vs 

for k in xrange(10000000):
    a = w2p_long(100000000000000)
    b = a+a
#5.0 sec

Perhaps it is not too important but I am worried it will slow down parsing of rows and pickling/unpickling of sessions containing rows.

Alan Etkin

unread,
Jun 23, 2013, 10:13:00 AM6/23/13
to web2py-d...@googlegroups.com
Perhaps it is not too important but I am worried it will slow down parsing of rows and pickling/unpickling of sessions containing rows.

I don't really think we need the overwriting of __repr__, however, in case it is implemented, I would suggest to use the case convention for classes in Python (something like Web2pyLong I suppose). I know you are aware of the notation but just in case.

Besides, it might happen that users could actually expect the row integer __repr__ to behave like in the standard Python object. So we would solve one small issue while opening another one.

Anthony

unread,
Jun 23, 2013, 10:14:54 AM6/23/13
to web2py-d...@googlegroups.com
Try:

for k in xrange(10000000):
    a
= long(100000000000000)
    b
= a+a

which is more like what the DAL actually does. In that case, I think the times are about the same. The overhead seems to be in calling the builtin function.

Anthony

Massimo DiPierro

unread,
Jun 23, 2013, 12:55:34 PM6/23/13
to web2py-d...@googlegroups.com
You are correct.

What do others think about this? If there is support, I have no serious objection.

Massimo

Anthony

unread,
Jun 23, 2013, 1:54:36 PM6/23/13
to web2py-d...@googlegroups.com
On Sunday, June 23, 2013 10:13:00 AM UTC-4, Alan Etkin wrote:
Perhaps it is not too important but I am worried it will slow down parsing of rows and pickling/unpickling of sessions containing rows.

I don't really think we need the overwriting of __repr__, however, in case it is implemented, I would suggest to use the case convention for classes in Python (something like Web2pyLong I suppose). I know you are aware of the notation but just in case.

Good point. I think I went with lowercase because long itself is lowercase (being a built-in type).
 
Besides, it might happen that users could actually expect the row integer __repr__ to behave like in the standard Python object. So we would solve one small issue while opening another one.

Yes, that's one misgiving I have. I assume it's not too likely that any application code will really rely on the "L" being appended when __repr__ is applied, but if someone is doing some debugging, they might be confused by seeing values printed without the "L" that are not simple int's. Not sure how big an issue this would be, and I suppose the documentation could clear it up.

I don't have a particularly strong opinion either way. Some folks seem to be bothered by the mid-stream switch to long's, and this seems like a way to stick with long's but resolve the serialization problem. I guess there's also the option of making it configurable via an argument to DAL().

Anthony

villas

unread,
Jun 23, 2013, 5:49:09 PM6/23/13
to web2py-d...@googlegroups.com
Maybe I don't understand this,  but doesn't python automatically promote int to long whenever it needs to do it?

Therefore,  those folks that break into the 'long' territory can write their own solution to whatever problem that may cause. 

The rest of us have been happy using ints so why even think about causing a backwards compatibility issue?

This reminds me of the bigint DAL issue.  In the end it was acknowledged that some people need the bigints,  and they can now have them,  adn that's great.  The rest of us continued using normal ints by default and I have hardly heard a word of confusion since.  If we are changing the framework just to cover a corner case,  we should maybe ask why?

Reply all
Reply to author
Forward
0 new messages