Merge lp:~agilebg/hr-timesheet/imp_hr_attendance_analysis_roundings into lp:~hr-core-editors/hr-timesheet/7.0

Proposed by Lorenzo Battistini
Status: Merged
Merged at revision: 66
Proposed branch: lp:~agilebg/hr-timesheet/imp_hr_attendance_analysis_roundings
Merge into: lp:~hr-core-editors/hr-timesheet/7.0
Diff against target: 33 lines (+13/-4)
1 file modified
hr_attendance_analysis/hr_attendance.py (+13/-4)
To merge this branch: bzr merge lp:~agilebg/hr-timesheet/imp_hr_attendance_analysis_roundings
Reviewer Review Type Date Requested Status
Pedro Manuel Baeza code review Approve
Yannick Vaucher @ Camptocamp code review, no test Approve
Joël Grand-Guillaume @ camptocamp code review, no tests Approve
Review via email: mp+206493@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Joël Grand-Guillaume @ camptocamp (jgrandguillaume-c2c) wrote :

LGTM, thanks !

review: Approve (code review, no tests)
Revision history for this message
Pedro Manuel Baeza (pedro.baeza) wrote :

Hi, Lorenzo,

Please use correct indentation in if sentence for PEP8 compliance (higher than nested block).

Functionally speaking, it seems good, but maybe you can do:

if (float_end_time - float_start_time) < 0.0000001:

and simplify the condition.

Regards.

review: Needs Fixing (code review)
Revision history for this message
Yannick Vaucher @ Camptocamp (yvaucher-c2c) wrote :

Hello,

I think you can even use float_compare instead of float_is_zero. That way you can do the whole teste in one if.

http://bazaar.launchpad.net/~openerp/openobject-server/7.0/view/head:/openerp/tools/float_utils.py#L99

if float_compare(float_end_time, float_start_time, precision_rounding=0.0000001) == -1:

Cheers.

review: Needs Fixing
63. By Lorenzo Battistini

[IMP] PEP8

Revision history for this message
Lorenzo Battistini (elbati) wrote :

Hello Pedro and Yannick,
I made the changes.
Thanks

64. By Lorenzo Battistini

[IMP] using float_compare

Revision history for this message
Yannick Vaucher @ Camptocamp (yvaucher-c2c) wrote :

LGTM

thanks!

review: Approve (code review, no test)
Revision history for this message
Pedro Manuel Baeza (pedro.baeza) wrote :

Thanks for the changes.

Regards.

review: Approve (code review)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'hr_attendance_analysis/hr_attendance.py'
2--- hr_attendance_analysis/hr_attendance.py 2013-11-25 08:21:41 +0000
3+++ hr_attendance_analysis/hr_attendance.py 2014-05-21 17:38:28 +0000
4@@ -26,6 +26,7 @@
5 import time
6 from datetime import *
7 import math
8+from openerp.tools import float_compare
9
10 import pytz
11
12@@ -75,10 +76,18 @@
13 return (td.microseconds + (td.seconds + td.days * 24 * 3600) * 10**6) / 10**6
14
15 def time_difference(self, float_start_time, float_end_time):
16- if float_end_time < float_start_time:
17- raise orm.except_orm(_('Error'), _('End time %s < start time %s')
18- % (str(float_end_time),str(float_start_time)))
19- delta = self.float_to_datetime(float_end_time) - self.float_to_datetime(
20+ if float_compare(
21+ float_end_time, float_start_time, precision_rounding=0.0000001
22+ ) == -1:
23+ # that means a difference smaller than 0.36 milliseconds
24+ raise orm.except_orm(
25+ _('Error'),
26+ _('End time %s < start time %s') % (
27+ str(float_end_time), str(float_start_time)
28+ )
29+ )
30+ delta = self.float_to_datetime(
31+ float_end_time) - self.float_to_datetime(
32 float_start_time)
33 return self.total_seconds(delta) / 60.0 / 60.0
34

Subscribers

People subscribed via source and target branches