[IMP] base_import: parsing/cleanup of floats

* Handle a leading + in import data, some contexts (e.g. bank
  statements) will mark positive sums explicitly for clarity
* Add basic grouping/decimal separator inference for people importing
  data from many localisations or to avoid them *having* to configure
  their separators if we can handle it for them, currently very basic

5 files changed, 59 insertions, 4 deletions

diff --git a/addons/base_import/models/base_import.py b/addons/base_import/models/base_import.py
index 2810ec46342..2a2af03bc35 100644
--- a/addons/base_import/models/base_import.py
+++ b/addons/base_import/models/base_import.py
@@ -2,6 +2,9 @@
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
 import base64
+import collections
+import unicodedata
+
 import chardet
 import datetime
 import io
@@ -633,7 +636,7 @@ class Import(models.TransientModel):
         if value.startswith('(') and value.endswith(')'):
             value = value[1:-1]
             negative = True
-        float_regex = re.compile(r'([-]?[0-9.,]+)')
+        float_regex = re.compile(r'([+-]?[0-9.,]+)')
         split_value = [g for g in float_regex.split(value) if g]
         if len(split_value) > 2:
             # This is probably not a float
@@ -656,18 +659,46 @@ class Import(models.TransientModel):
 
     @api.model
     def _parse_float_from_data(self, data, index, name, options):
-        thousand_separator = options.get('float_thousand_separator', ' ')
-        decimal_separator = options.get('float_decimal_separator', '.')
         for line in data:
             line[index] = line[index].strip()
             if not line[index]:
                 continue
+            thousand_separator, decimal_separator = self._infer_separators(line[index], options)
             line[index] = line[index].replace(thousand_separator, '').replace(decimal_separator, '.')
             old_value = line[index]
             line[index] = self._remove_currency_symbol(line[index])
             if line[index] is False:
                 raise ValueError(_("Column %s contains incorrect values (value: %s)" % (name, old_value)))
 
+    def _infer_separators(self, value, options):
+        """ Try to infer the shape of the separators: if there are two
+        different "non-numberic" characters in the number, the
+        former/duplicated one would be grouping ("thousands" separator) and
+        the latter would be the decimal separator. The decimal separator
+        should furthermore be unique.
+        """
+        # can't use \p{Sc} using re so handroll it
+        non_number = [
+            # any character
+            c for c in value
+            # which is not a numeric decoration (() is used for negative
+            # by accountants)
+            if c not in '()-+'
+            # which is not a digit or a currency symbol
+            if unicodedata.category(c) not in ('Nd', 'Sc')
+        ]
+
+        counts = collections.Counter(non_number)
+        # if we have two non-numbers *and* the last one has a count of 1,
+        # we probably have grouping & decimal separators
+        if len(counts) == 2 and counts[non_number[-1]] == 1:
+            return [character for character, _count in counts.most_common()]
+
+        # otherwise get whatever's in the options, or fallback to a default
+        thousand_separator = options.get('float_thousand_separator', ' ')
+        decimal_separator = options.get('float_decimal_separator', '.')
+        return thousand_separator, decimal_separator
+
     @api.multi
     def _parse_import_data(self, data, import_fields, options):
         """ Lauch first call to _parse_import_data_recursive with an
diff --git a/addons/base_import/models/test_models.py b/addons/base_import/models/test_models.py
index 91aa8611a1c..de5293c2d3d 100644
--- a/addons/base_import/models/test_models.py
+++ b/addons/base_import/models/test_models.py
@@ -74,3 +74,10 @@ class PreviewModel(models.Model):
     name = fields.Char('Name')
     somevalue = fields.Integer(string='Some Value', required=True)
     othervalue = fields.Integer(string='Other Variable')
+
+class FloatModel(models.Model):
+    _name = name('float')
+
+    value = fields.Float()
+    value2 = fields.Monetary()
+    currency_id = fields.Many2one('res.currency')
diff --git a/addons/base_import/security/ir.model.access.csv b/addons/base_import/security/ir.model.access.csv
index 6844f024d3e..84e83b2d71c 100644
--- a/addons/base_import/security/ir.model.access.csv
+++ b/addons/base_import/security/ir.model.access.csv
@@ -11,6 +11,7 @@ access_base_import_tests_models_m2o_required,base.import.tests.models.m2o.requir
 access_base_import_tests_models_m2o_required_related,base.import.tests.models.m2o.required.related,model_base_import_tests_models_m2o_required_related,base.group_user,1,1,1,1
 access_base_import_tests_models_o2m,base.import.tests.models.o2m,model_base_import_tests_models_o2m,base.group_user,1,1,1,1
 access_base_import_tests_models_o2m_child,base.import.tests.models.o2m.child,model_base_import_tests_models_o2m_child,base.group_user,1,1,1,1
+access_base_import_tests_models_float,base.import.tests.models.float,model_base_import_tests_models_float,base.group_user,1,1,1,1
 access_base_import_tests_models_preview,base.import.tests.models.preview,model_base_import_tests_models_preview,base.group_user,1,1,1,1
 access_base_import_mapping,base.import.mapping,model_base_import_mapping,base.group_user,1,1,1,1
 
diff --git a/addons/base_import/tests/test_base_import.py b/addons/base_import/tests/test_base_import.py
index 3f5830a8db2..bb95bc52699 100644
--- a/addons/base_import/tests/test_base_import.py
+++ b/addons/base_import/tests/test_base_import.py
@@ -452,6 +452,23 @@ class test_convert_import_data(TransactionCase):
         self.assertEqual(float(result[0][-1]), 5.69)
         self.assertEqual(str(result[0][-2]), '2017-10-12')
 
+    def test_parse_float(self):
+        w = self.env['base_import.import'].create({
+            'res_model': 'base_import.tests.models.float',
+        })
+        data = w._parse_import_data(
+            [
+                ['1.62'], ['-1.62'], ['+1.62'], ['  +1.62  '], ['(1.62)'],
+                ["1'234'567,89"], ["1.234.567'89"]
+            ],
+            ['value'], {}
+        )
+        self.assertEqual(
+            [d[0] for d in data],
+            ['1.62', '-1.62', '+1.62', '+1.62', '-1.62',
+             '1234567.89', '1234567.89']
+        )
+
     def test_filtered(self):
         """ If ``False`` is provided as field mapping for a column,
         that column should be removed from importable data
diff --git a/odoo/addons/test_impex/tests/test_load.py b/odoo/addons/test_impex/tests/test_load.py
index 628e879374d..2a301e8a8cd 100644
--- a/odoo/addons/test_impex/tests/test_load.py
+++ b/odoo/addons/test_impex/tests/test_load.py
@@ -334,7 +334,6 @@ class test_float_field(ImporterCase):
         self.assertEqual(result['messages'], [
             message(u"'foobar' does not seem to be a number for field 'Value'")])
 
-
 class test_string_field(ImporterCase):
     model_name = 'export.string.bounded'