Added comparison dunder methods and some tests #53
Conversation
distill/core/log.py
Outdated
| if isinstance(other, Log): | ||
| return self.id < other.id | ||
| if isinstance(other, datetime): | ||
| return self.id.get_datetime() < other.now() |
There was a problem hiding this comment.
other.now() will get the current datetime, I think instead we should just be able to compare self.id.get_datetime() < other if other is a datetime object.
There was a problem hiding this comment.
This assumes get_datetime() returns an actual datetime object, but I don't think it does. Tracking down the method starting from the PKSUID repo to the datetime library docs suggests it just returns a float.
Indeed, testing it in the terminal shows it just returns POSIX time float. I believe datetime objects have a timestamp() method that will return what we want. But we need to be careful to ensure get_datetime() and other.timestamp() are operating on the same scale.
There was a problem hiding this comment.
@EandrewJones looks to me like it does return a datetime object? "getDatetime() returns a python date object which represents the approximate time (missing microseconds) that the pksuid was created"
There was a problem hiding this comment.
I found that Ryan's comparison works, while trying to compare get_datetime() to timestamp() gives an error due to the float return value of timestamp.
distill/core/log.py
Outdated
| if isinstance(other, Log): | ||
| return self.id <= other.id | ||
| if isinstance(other, datetime): | ||
| return self.id.get_datetime() <= other.now() |
There was a problem hiding this comment.
Same as above for every method
There was a problem hiding this comment.
Response is the same as my comment above.
|
Also need to resolve conflicts. |
There was a problem hiding this comment.
Looks good to me - @EandrewJones or @Jyyjy can do final look over and merge
[Evan] Closes #43