From 36ee8a78ece08c3fd18ec2cd4904e464641e0b5d Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Tue, 7 Aug 2018 16:34:11 +0200 Subject: [PATCH] [IMP] base_import: attempt to automatically guess the separator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It would probably be even better to iterate the file content and get the non-quoted non-alphanumeric characters as separator candidates (instead of a hard-coded list) however Python does not seem to have a decoding iterator (taking bytes and yielding an iterator of codepoints or even grapheme clusters) — incidentally uniseg seems to require up-front decoding as well — so that's not really convenient as we may be dealing with large-ish files and not want to load it entirely in memory. An alternative would be to use TextIOWrapper and iterate the file by buffers of a few ks, and classify that based on either codepoints or grapheme clusters. --- addons/base_import/models/base_import.py | 24 ++++- .../base_import/security/ir.model.access.csv | 2 +- .../static/src/js/import_action.js | 2 +- addons/base_import/tests/test_csv_magic.py | 94 +++++++++++++++++-- 4 files changed, 111 insertions(+), 11 deletions(-) diff --git a/addons/base_import/models/base_import.py b/addons/base_import/models/base_import.py index 31de56d79e3..3055bcc9e3c 100644 --- a/addons/base_import/models/base_import.py +++ b/addons/base_import/models/base_import.py @@ -322,10 +322,30 @@ class Import(models.TransientModel): if encoding != 'utf-8': csv_data = csv_data.decode(encoding).encode('utf-8') + separator = options.get('separator') + if not separator: + # default for unspecified separator so user gets a message about + # having to specify it + separator = ',' + for candidate in (',', ';', '\t', ' ', '|', unicodedata.lookup('unit separator')): + # pass through the CSV and check if all rows are the same + # length & at least 2-wide assume it's the correct one + it = pycompat.csv_reader(io.BytesIO(csv_data), quotechar=options['quoting'], delimiter=candidate) + w = None + for row in it: + width = len(row) + if w is None: + w = width + if width == 1 or width != w: + break # next candidate + else: # nobreak + separator = options['separator'] = candidate + break + csv_iterator = pycompat.csv_reader( io.BytesIO(csv_data), - quotechar=str(options['quoting']), - delimiter=str(options['separator'])) + quotechar=options['quoting'], + delimiter=separator) return ( row for row in csv_iterator diff --git a/addons/base_import/security/ir.model.access.csv b/addons/base_import/security/ir.model.access.csv index 84e83b2d71c..06a166c91c9 100644 --- a/addons/base_import/security/ir.model.access.csv +++ b/addons/base_import/security/ir.model.access.csv @@ -14,4 +14,4 @@ access_base_import_tests_models_o2m_child,base.import.tests.models.o2m.child,mod 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 - +access_base_import_tests_models_complex,access_base_import_tests_models_complex,model_base_import_tests_models_complex,base.group_user,1,0,0,0 diff --git a/addons/base_import/static/src/js/import_action.js b/addons/base_import/static/src/js/import_action.js index 87a971d0dc5..6c28797161d 100644 --- a/addons/base_import/static/src/js/import_action.js +++ b/addons/base_import/static/src/js/import_action.js @@ -76,7 +76,7 @@ var DataImport = AbstractAction.extend(ControlPanelMixin, { template: 'ImportView', opts: [ {name: 'encoding', label: _lt("Encoding:"), value: ''}, - {name: 'separator', label: _lt("Separator:"), value: ','}, + {name: 'separator', label: _lt("Separator:"), value: ''}, {name: 'quoting', label: _lt("Text Delimiter:"), value: '"'} ], parse_opts: [ diff --git a/addons/base_import/tests/test_csv_magic.py b/addons/base_import/tests/test_csv_magic.py index 2b088c37afe..92724ec360b 100644 --- a/addons/base_import/tests/test_csv_magic.py +++ b/addons/base_import/tests/test_csv_magic.py @@ -6,7 +6,17 @@ import codecs from odoo.tests import common -class TestEncoding(common.TransactionCase): +class ImportCase(common.TransactionCase): + def _make_import(self, contents): + return self.env['base_import.import'].create({ + 'res_model': 'base_import.tests.models.complex', + 'file_name': 'f', + 'file_type': 'text/csv', + 'file': contents, + }) + + +class TestEncoding(ImportCase): """ create + parse_preview -> check result options """ @@ -47,13 +57,83 @@ class TestEncoding(common.TransactionCase): self.assertEqual(r['options']['encoding'], 'iso-8859-1') self.assertEqual(r['preview'], [['text'], [s.decode('iso-8859-1')]]) - def _make_import(self, contents): - return self.env['base_import.import'].create({ - 'res_model': 'base_import.tests.models.complex', - 'file_name': 'f', - 'file_type': 'text/csv', - 'file': contents, +class TestFileSeparator(ImportCase): + + def setUp(self): + super().setUp() + self.imp = self._make_import( +"""c|f +a|1 +b|2 +c|3 +d|4 +""") + + def test_explicit_success(self): + r = self.imp.parse_preview({ + 'separator': '|', + 'headers': True, + 'quoting': '"', }) + self.assertEqual(r['headers'], ['c', 'f']) + self.assertEqual(r['preview'], [ + ['a', '1'], + ['b', '2'], + ['c', '3'], + ['d', '4'], + ]) + self.assertEqual(r['options']['separator'], '|') + + def test_explicit_fail(self): + """ Don't protect user against making mistakes + """ + r = self.imp.parse_preview({ + 'separator': ',', + 'headers': True, + 'quoting': '"', + }) + self.assertEqual(r['headers'], ['c|f']) + self.assertEqual(r['preview'], [ + ['a|1'], + ['b|2'], + ['c|3'], + ['d|4'], + ]) + self.assertEqual(r['options']['separator'], ',') + + def test_guess_ok(self): + r = self.imp.parse_preview({ + 'separator': '', + 'headers': True, + 'quoting': '"', + }) + self.assertEqual(r['headers'], ['c', 'f']) + self.assertEqual(r['preview'], [ + ['a', '1'], + ['b', '2'], + ['c', '3'], + ['d', '4'], + ]) + self.assertEqual(r['options']['separator'], '|') + + def test_noguess(self): + """ If the guesser has no idea what the separator is, it defaults to + "," but should not set that value + """ + imp = self._make_import('c\na\nb\nc\nd') + r = imp.parse_preview({ + 'separator': '', + 'headers': True, + 'quoting': '"', + }) + self.assertEqual(r['headers'], ['c']) + self.assertEqual(r['preview'], [ + ['a'], + ['b'], + ['c'], + ['d'], + ]) + self.assertEqual(r['options']['separator'], '') class TestNumberSeparators(common.TransactionCase): def test_parse_float(self):