Skip to content

feature property types ✨ #16

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

feature property types ✨ #16

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
30f06de
feature property types :sparkles:
Oct 29, 2022
32f526b
implemented dateNano
Oct 30, 2022
efb7779
implemented requested PR changes
liquidiert Nov 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions example/__main__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,16 @@
from cmd import Cmd
import objectbox
import datetime
from datetime import datetime
from example.model import *


# objectbox expects date timestamp in milliseconds since UNIX epoch
def now_ms() -> int:
seconds: float = datetime.datetime.utcnow().timestamp()
return round(seconds * 1000)
return datetime.utcnow()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is much more readable. Does that also have similar (millisecond) precision?

Btw, The docs say that datetime.now(timezone.utc) is preferred for some reason, but I did not dig deeper.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docs say this because of:

Because naive datetime objects are treated by many datetime methods as local times, it is preferred to use aware datetimes to represent times in UTC. As such, the recommended way to create an object representing the current time in UTC is by calling datetime.now(timezone.utc).

Gonna change it 👍



def format_date(timestamp_ms: int) -> str:
return "" if timestamp_ms == 0 else str(datetime.datetime.fromtimestamp(timestamp_ms / 1000))
def format_date(timestamp_ms: datetime) -> str:
return "" if timestamp_ms == 0 else str(timestamp_ms)


class TasklistCmd(Cmd):
Expand Down
6 changes: 3 additions & 3 deletions example/model.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from datetime import datetime
from objectbox.model import *


Expand All @@ -6,9 +7,8 @@ class Task:
id = Id(id=1, uid=1001)
text = Property(str, id=2, uid=1002)

# TODO property type DATE
date_created = Property(int, id=3, uid=1003)
date_finished = Property(int, id=4, uid=1004)
date_created = Property(datetime, type=PropertyType.dateNano, id=3, uid=1003)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, one TODO less! 👍

Btw, using it in the example is great, and a test would be even better (optionally only if you want...)

date_finished = Property(datetime, id=4, uid=1004)


def get_objectbox_model():
Expand Down
2 changes: 1 addition & 1 deletion objectbox/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
]

# Python binding version
version = Version(0, 4, 0)
version = Version(0, 5, 0)


def version_info():
Expand Down
1 change: 1 addition & 0 deletions objectbox/c.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,7 @@ def c_voidp_as_bytes(voidp, size):
OBXPropertyType_String = 9
OBXPropertyType_Date = 10
OBXPropertyType_Relation = 11
OBXPropertyType_DateNano = 12
OBXPropertyType_ByteVector = 23
OBXPropertyType_StringVector = 30

Expand Down
10 changes: 8 additions & 2 deletions objectbox/model/entity.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
# limitations under the License.


from datetime import datetime
import flatbuffers
from objectbox.c import *
from objectbox.model.properties import Property
Expand Down Expand Up @@ -78,8 +79,10 @@ def fill_properties(self):
def get_value(self, object, prop: Property):
# in case value is not overwritten on the object, it's the Property object itself (= as defined in the Class)
val = getattr(object, prop._name)
if val == prop:
return prop._py_type() # default (empty) value for the given type
if isinstance(val, datetime): # handle datetimes first
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not very familiar with specifics here; maybe we can avoid isinstance here and go via property type? I am assuming that isinstance takes more CPU cycles as it is common for reflective calls for the languages I know, but I am not familiar with Python internals.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed to use hasttr; property type is ambiguous unfortunately if val is the property itself

return int(val.timestamp()) if prop._ob_type == OBXPropertyType_Date else val.timestamp()
elif val == prop:
return prop._py_type() if not hasattr(prop._py_type, "timestamp") else 0 # default (empty) value for the given type
return val

def get_object_id(self, object) -> int:
Expand Down Expand Up @@ -143,6 +146,9 @@ def unmarshal(self, data: bytes):

# slice the vector as a requested type
val = prop._py_type(table.Bytes[start:start+size])
elif prop._ob_type == OBXPropertyType_Date or prop._ob_type == OBXPropertyType_DateNano:
table_val = table.Get(prop._fb_type, o + table.Pos)
val = datetime.fromtimestamp(table_val) if table_val != 0 else 0 # default timestamp
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you would need to differentiate Date vs. DateNano here? E.g. a factor of 1000000?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If floats are used this happens automatically

else:
val = table.Get(prop._fb_type, o + table.Pos)

Expand Down
9 changes: 7 additions & 2 deletions objectbox/model/properties.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.

from datetime import datetime
from enum import IntEnum

from objectbox.c import *
Expand All @@ -28,7 +29,8 @@ class PropertyType(IntEnum):
float = OBXPropertyType_Float
double = OBXPropertyType_Double
string = OBXPropertyType_String
# date = OBXPropertyType_Date
date = OBXPropertyType_Date
dateNano = OBXPropertyType_DateNano
# relation = OBXPropertyType_Relation
byteVector = OBXPropertyType_ByteVector
# stringVector = OBXPropertyType_StringVector
Expand All @@ -44,7 +46,8 @@ class PropertyType(IntEnum):
PropertyType.float: flatbuffers.number_types.Float32Flags,
PropertyType.double: flatbuffers.number_types.Float64Flags,
PropertyType.string: flatbuffers.number_types.UOffsetTFlags,
# PropertyType.date: flatbuffers.number_types.Int64Flags,
PropertyType.date: flatbuffers.number_types.Int64Flags,
PropertyType.dateNano: flatbuffers.number_types.Float64Flags,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Date nano also use 64 bit integers; there should not be any floats involved?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry thought for the extra resolution of nano seconds this would be necessary

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Internally (e.g. FlatBuffers wise) it's ints and the resolution is "perfect" for a limited time (a couple of 200+ years left iirc) - I do not know about Python though and how to treat nano precision there...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm I would see it as a little hack.
When I have a timestamp in nanosecond resolution e.g. 1667401463110012130 I'd have to divide it by 1.000.000.000 before I can use it in .fromtimestamp(). But the division is not different to using a float value in the first place. It might be even better to use floats to preserve precision cause:

>>> 1667401463110012130 / 1000000000
1667401463.110012

results in a precision loss. What do you think @greenrobot ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The flatbuffers representation should not store floating point for date nano to match the other platforms. (binary compatibility, also because of sync)

# PropertyType.relation: flatbuffers.number_types.Int64Flags,
PropertyType.byteVector: flatbuffers.number_types.UOffsetTFlags,
# PropertyType.stringVector: flatbuffers.number_types.UOffsetTFlags,
Expand Down Expand Up @@ -81,6 +84,8 @@ def __determine_ob_type(self) -> OBXPropertyType:
return OBXPropertyType_Double
elif ts == bool:
return OBXPropertyType_Bool
elif ts == datetime:
return OBXPropertyType_Date
else:
raise Exception("unknown property type %s" % ts)

Expand Down