I would rename the module to export_sage50 or accounting_export_sage50 Code issues: exportsage50/wizard/exportsage50.py:61:16: missing context=context exportsage50/wizard/exportsage50.py:61:16: no checking if there are any results, can lead to index errors exportsage50/wizard/exportsage50.py:61:17: no checking if there are any results, can lead to index errors exportsage50/wizard/exportsage50.py:120:51: or "" has no effect due to use of str() (str(None) -> 'None') exportsage50/wizard/exportsage50.py:134:37: the list generator here is pointless just use ','.join(fields_sale_invoice), it is also EXTREMELY hard to read your choice of variable names exportsage50/wizard/exportsage50.py:140:27: search without context=context exportsage50/wizard/exportsage50.py:143:32: browse without context=context exportsage50/wizard/exportsage50.py:148:48: precision_get without context=context exportsage50/wizard/exportsage50.py:167:63: the list generator here is pointless just use ','.join(fields_sale_invoice), it is also EXTREMELY hard to read your choice of variable names Minor code / Style issues: exportsage50/wizard/exportsage50.py:30:5: Content of docstring is pointless exportsage50/wizard/exportsage50.py:35:5: Commented out code exportsage50/wizard/exportsage50.py:44:46: Excessively long line, consider breaking at 120 chars. exportsage50/wizard/exportsage50.py:44:48: Not sure what use lambda is here, you don't need it exportsage50/wizard/exportsage50.py:51:5: Method can be static exportsage50/wizard/exportsage50.py:52:5: Commented out code exportsage50/wizard/exportsage50.py:55:5: Method can be static exportsage50/wizard/exportsage50.py:55:5: *args exportsage50/wizard/exportsage50.py:58:5: Monolithic function, consider breaking it down into multiple smaller functions exportsage50/wizard/exportsage50.py:68:9: French comment exportsage50/wizard/exportsage50.py:73:13: French comment exportsage50/wizard/exportsage50.py:75:13: French comment exportsage50/wizard/exportsage50.py:77:13: mix of CamelCase and underline_case, pick one and be consistent exportsage50/wizard/exportsage50.py:89:13: French comment exportsage50/wizard/exportsage50.py:90:13: Shadowing 'fields' from openerp.osv.fields exportsage50/wizard/exportsage50.py:94:13: Commented out code exportsage50/wizard/exportsage50.py:95:13: Commented out code exportsage50/wizard/exportsage50.py:97:13: French comment exportsage50/wizard/exportsage50.py:102:13: French comment exportsage50/wizard/exportsage50.py:104:101: Comment could be more descriptive, as it is now, it is redundant to someone reading your code, I would also put it a line above instead of at the end of the line exportsage50/wizard/exportsage50.py:107:13: French comment exportsage50/wizard/exportsage50.py:109:13: French comment exportsage50/wizard/exportsage50.py:111,114-116,119: You should be able to directly apply max() to line.payment_ids exportsage50/wizard/exportsage50.py:116:17: mix of CamelCase and underline_case, pick one and be consistent exportsage50/wizard/exportsage50.py:117:17: French comment exportsage50/wizard/exportsage50.py:118:41: pool.get is an expensive function that always returns the same result, don't call it inside of a loop, call it at the beginning of your function exportsage50/wizard/exportsage50.py:120:17: French variable name exportsage50/wizard/exportsage50.py:135:13: Commented out code exportsage50/wizard/exportsage50.py:137:17: French variable name (taxe) exportsage50/wizard/exportsage50.py:139:40: pool.get is an expensive function that always returns the same result, don't call it inside of a loop, call it at the beginning of your function exportsage50/wizard/exportsage50.py:148:48: pool.get is an expensive function that always returns the same result, don't call it inside of a DOUBLE loop, call it at the beginning of your function exportsage50/wizard/exportsage50.py:149-150: this code can be simplified to one line exportsage50/wizard/exportsage50.py:151:21: Commented out code exportsage50/wizard/exportsage50.py:154:21: French comment exportsage50/wizard/exportsage50.py:156:17: French variable name (taxe) exportsage50/wizard/exportsage50.py:159:33: I would just like to point out that you're in your 8th indentation (for: if: for: if: for: if). This is _VERY_ bad coding style and I would block your MP based just on this. exportsage50/wizard/exportsage50.py:165-166: Could be one line exportsage50/wizard/exportsage50.py:168:29: Commented out code exportsage50/wizard/exportsage50.py:169:25: Commented out code exportsage50/wizard/exportsage50.py:172:17: Commented out code exportsage50/wizard/exportsage50.py:174:13: French comment exportsage50/wizard/exportsage50.py:175:13: Commented out code exportsage50/wizard/exportsage50.py:177:9: Commented out code Spelling: exportsage50/wizard/exportsage50.py:65:85: Sentence fragments, no need for a period here. exportsage50/wizard/exportsage50.py:93:20: costumer -> customer; costumer doesn't mean what you think it means Formatting: exportsage50/__openerp__.py:36:1: this is a sub-header, you should use '-' instead of '=' exportsage50/__openerp__.py:43:1: this is a sub-header, you should use '-' instead of '=' exportsage50/__openerp__.py:46:1: missing linebreak or *, the way it is now will put 45 and 46 on the same line exportsage50/__openerp__.py:100:1: this is a sub-header, you should use '-' instead of '=' exportsage50/__openerp__.py:102:1: this is a list, should start with * exportsage50/__openerp__.py:103:1: this is a list, should start with *