-
Notifications
You must be signed in to change notification settings - Fork 1
Make compatible with python 2 and python 3 #44
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
base: master
Are you sure you want to change the base?
Changes from all commits
6f48603
fd1db8a
9a82719
981ab9a
64e16d2
2a44caa
d972b79
538b938
9af04ec
92aecb3
b6bd4cd
3a1f6ba
b6d4ffe
86e9cc6
b735f5b
2db729f
c6c0d77
94029b3
b6f3dbe
3886461
92f3f81
b9ad574
666a0aa
1eb0c34
1db4387
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
from builtins import str | ||
import pymongo | ||
import warnings | ||
|
||
|
@@ -43,11 +44,11 @@ def save( self, forceInsert=False, **kwargs ): | |
newId = collection.insert( self._data, **kwargs ) | ||
else: | ||
newId = collection.save( self._data, **kwargs ) | ||
except pymongo.errors.OperationFailure, err: | ||
except pymongo.errors.OperationFailure as err: | ||
message = 'Could not save document (%s)' | ||
if u'duplicate key' in unicode(err): | ||
if u'duplicate key' in str(err): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Of all the things Python 2.x. did horribly, strings are definitely at the top of the list! :P I would highly suggest including the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I ended up using |
||
message = u'Tried to save duplicate unique keys (%s)' | ||
raise OperationError( message % unicode(err) ) | ||
raise OperationError( message % str(err) ) | ||
if newId is not None: | ||
setattr(self, self._primaryKeyField, newId) | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
from builtins import object | ||
class DocumentRegistry(object): | ||
documentTypes = {} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
from future.types.newobject import newobject | ||
Juko marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
def serialiseTypesForDocumentType( documentType ): | ||
return [ cls.__name__ for cls in documentType.mro() if cls != object \ | ||
return [ cls.__name__ for cls in documentType.mro() if cls not in [object, newobject] \ | ||
and cls.__name__ not in ['Document', 'BaseDocument', 'EmbeddedDocument'] ] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
from builtins import object | ||
class BaseField(object): | ||
_resyncAtSave = False | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,15 @@ | ||
from builtins import str | ||
|
||
from mongorm.fields.BaseField import BaseField | ||
|
||
|
||
class StringField(BaseField): | ||
def fromPython( self, pythonValue, dereferences=[], modifier=None ): | ||
if pythonValue is not None: | ||
pythonValue = unicode(pythonValue) | ||
pythonValue = str(pythonValue) | ||
return pythonValue | ||
|
||
def toPython( self, bsonValue ): | ||
if bsonValue is not None: | ||
bsonValue = unicode(bsonValue) | ||
bsonValue = str(bsonValue) | ||
return bsonValue |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
objects are not truthy by default?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Neither in python 2 or 3 are empty objects considered
truthy
. But because the file overwrites__getattr__
(so that it returnsNone
instead of anAttributeError
), the way thefuture
object
class checks truthiness makes it error because it believes the attribute (the__bool__
function) exists on itself, but when it calls it, it dies. But the old behaviour of the library is that an empty Document is truthy, so this just keeps that compatibilityThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
damn
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That sounds like a bug;
__getattr__
should raise anAttributeError
for things that aren't fields. Actually all objects in Python are consideredTrue
unless they specify a__bool__
method which returnsFalse
or__len__
method which returns 0. Also if you decide to go ahead with this workaround you should probably also specify a__nonzero__
method for Python 2.x (__bool__
was added in Python 3.x)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I don't like that
__getattr__
doesn't raise anAttributeError
either ahaha. But it's been that way in the library since it's inception it seems, so I don't want to break functionality as core as that in OpenLearning which expects it. After a better inspection across the whole OpenLearning codebase to check it's uses, I'd be more comfortable to recommend that we make that change!Yeap, whoops. Working on JS my brain confused object and dict. Yeap, it's not that
object
isfalsy
, it's that thefuture
version ofobject
that we're importing (akafrom builtins import object
) first checks if the__bool__
method is available and uses that to determine it's truthiness (because it defines the__nonzero__
method) that python 2 uses. But because of the aboveAttributeError
not being raised, it tries to call that method, and dies because it doesn't actually exist to be called. So this was just there to short circuit it into an acceptable response that we want!