Code review comment for lp:~arthru/prestashoperpconnect/import-product-combinations

Good piece of work Arthur,

Some comments:

--

What is a "product combination". At least a few words (in the docstring of the Python module product_combination.py) to explain this concept could be useful.

--

l.44-47, l.217ff. l.226ff (and other places, possibly on methods search, browse, read, create, unlink)

    model = self.session.pool.get('product.product')
    product_ids = model.search(self.session.cr, self.session.uid, [
        ('default_code', '=', code)
    ])

A better idiom is:

    product_ids = self.session.search('product.product', [('default_code', '=', code)])

(side note on the alignment: "Arguments on first line forbidden when not using vertical alignment", source http://www.python.org/dev/peps/pep-0008/#indentation)

--

l.134
"unidecode" is to add in the "external_dependencies" in __openerp__.py

--

l.212 (and each time you use the unwrap keyword argument of to_openerp())

Please use the keyword when calling a keyword argument, so instead of

    attribute_id = option_binder.to_openerp(
        option_value['id_attribute_group'], True)

Use

    attribute_id = option_binder.to_openerp(
        option_value['id_attribute_group'], unwrap=True)

--
l.264, l.302, l.453

Instead of:

  type(main_product[attribute]) is list

Use:

  isinstance(main_product[attribute], list)

--
l.306-321:
suggestion (as you want): externalize the part which get the option value in another ConnectorUnit, allowing to customize the way the options values are get.

--
l.599
    if len(combinations) == 0:
=>
    if not combinations:

--

Thanks!