From 9a4f1a76fd227d8001b913b9206c608840edb94e Mon Sep 17 00:00:00 2001 From: Jeff Tratner Date: Thu, 25 May 2017 21:35:00 -0700 Subject: [PATCH 1/3] BUG: Fix uninformative error on bad CSV engine name Previously had an UnboundLocalError - no fun! --- doc/source/whatsnew/v0.20.2.txt | 3 +++ pandas/io/parsers.py | 3 +++ pandas/tests/io/parser/test_parsers.py | 12 ++++++++++++ 3 files changed, 18 insertions(+) diff --git a/doc/source/whatsnew/v0.20.2.txt b/doc/source/whatsnew/v0.20.2.txt index 13365401f1d1c..653f04d4dbee7 100644 --- a/doc/source/whatsnew/v0.20.2.txt +++ b/doc/source/whatsnew/v0.20.2.txt @@ -39,6 +39,9 @@ Bug Fixes - Bug in using ``pathlib.Path`` or ``py.path.local`` objects with io functions (:issue:`16291`) - Bug in ``DataFrame.update()`` with ``overwrite=False`` and ``NaN values`` (:issue:`15593`) +- Passing an invalid engine to :func:`read_csv` now raises an informative + ValueError rather than UnboundLocalError. (:issue:`16511`) + diff --git a/pandas/io/parsers.py b/pandas/io/parsers.py index e287d92f67ef6..92ff009caebc0 100755 --- a/pandas/io/parsers.py +++ b/pandas/io/parsers.py @@ -969,6 +969,9 @@ def _make_engine(self, engine='c'): klass = PythonParser elif engine == 'python-fwf': klass = FixedWidthFieldParser + else: + raise ValueError('Unknown engine: %r (valid are "c", "python",' + ' or "python-fwf")' % engine) self._engine = klass(self.f, **self.options) def _failover_to_python(self): diff --git a/pandas/tests/io/parser/test_parsers.py b/pandas/tests/io/parser/test_parsers.py index 8d59e3acb3230..cc9a443f76871 100644 --- a/pandas/tests/io/parser/test_parsers.py +++ b/pandas/tests/io/parser/test_parsers.py @@ -2,6 +2,8 @@ import os +import pytest + import pandas.util.testing as tm from pandas import read_csv, read_table @@ -99,3 +101,13 @@ def read_table(self, *args, **kwds): kwds = kwds.copy() kwds['engine'] = self.engine return read_table(*args, **kwds) + + +class TestParameterValidation(object): + def test_unknown_engine(self): + with tm.ensure_clean() as path: + df = tm.makeDataFrame() + df.to_csv(path) + with pytest.raises(ValueError) as exc_info: + read_csv(path, engine='pyt') + exc_info.match('Unknown engine') From 35968dc6e1719859c222ff41924ef6d106e81961 Mon Sep 17 00:00:00 2001 From: Jeff Tratner Date: Fri, 26 May 2017 17:04:37 -0700 Subject: [PATCH 2/3] Move things around as per requested --- pandas/io/parsers.py | 5 +++-- pandas/tests/io/parser/common.py | 8 ++++++++ pandas/tests/io/parser/test_parsers.py | 12 ------------ 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/pandas/io/parsers.py b/pandas/io/parsers.py index 92ff009caebc0..12b606d969c7d 100755 --- a/pandas/io/parsers.py +++ b/pandas/io/parsers.py @@ -970,8 +970,9 @@ def _make_engine(self, engine='c'): elif engine == 'python-fwf': klass = FixedWidthFieldParser else: - raise ValueError('Unknown engine: %r (valid are "c", "python",' - ' or "python-fwf")' % engine) + raise ValueError('Unknown engine: {engine} (valid options are' + ' "c", "python", or' ' "python-fwf")'.format( + engine=engine)) self._engine = klass(self.f, **self.options) def _failover_to_python(self): diff --git a/pandas/tests/io/parser/common.py b/pandas/tests/io/parser/common.py index 31d815a4bca97..8c7bb4df864c1 100644 --- a/pandas/tests/io/parser/common.py +++ b/pandas/tests/io/parser/common.py @@ -11,6 +11,7 @@ import pytest import numpy as np +from pandas import read_csv from pandas._libs.lib import Timestamp import pandas as pd @@ -1754,3 +1755,10 @@ def test_skip_bad_lines(self): val = sys.stderr.getvalue() assert 'Skipping line 3' in val assert 'Skipping line 5' in val + + def test_unknown_engine(self): + with tm.ensure_clean() as path: + df = tm.makeDataFrame() + df.to_csv(path) + with tm.assert_raises_regex(ValueError, 'Unknown engine'): + read_csv(path, engine='pyt') diff --git a/pandas/tests/io/parser/test_parsers.py b/pandas/tests/io/parser/test_parsers.py index cc9a443f76871..8d59e3acb3230 100644 --- a/pandas/tests/io/parser/test_parsers.py +++ b/pandas/tests/io/parser/test_parsers.py @@ -2,8 +2,6 @@ import os -import pytest - import pandas.util.testing as tm from pandas import read_csv, read_table @@ -101,13 +99,3 @@ def read_table(self, *args, **kwds): kwds = kwds.copy() kwds['engine'] = self.engine return read_table(*args, **kwds) - - -class TestParameterValidation(object): - def test_unknown_engine(self): - with tm.ensure_clean() as path: - df = tm.makeDataFrame() - df.to_csv(path) - with pytest.raises(ValueError) as exc_info: - read_csv(path, engine='pyt') - exc_info.match('Unknown engine') From 7c5f2c4d4c812ffd870b601df7a053f7b68788d8 Mon Sep 17 00:00:00 2001 From: Jeff Tratner Date: Fri, 26 May 2017 17:15:10 -0700 Subject: [PATCH 3/3] Move one more time --- doc/source/whatsnew/v0.20.2.txt | 2 +- pandas/tests/io/parser/common.py | 8 -------- pandas/tests/io/test_common.py | 7 +++++++ 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/doc/source/whatsnew/v0.20.2.txt b/doc/source/whatsnew/v0.20.2.txt index 653f04d4dbee7..48c2d772ab35f 100644 --- a/doc/source/whatsnew/v0.20.2.txt +++ b/doc/source/whatsnew/v0.20.2.txt @@ -40,7 +40,7 @@ Bug Fixes - Bug in using ``pathlib.Path`` or ``py.path.local`` objects with io functions (:issue:`16291`) - Bug in ``DataFrame.update()`` with ``overwrite=False`` and ``NaN values`` (:issue:`15593`) - Passing an invalid engine to :func:`read_csv` now raises an informative - ValueError rather than UnboundLocalError. (:issue:`16511`) + ``ValueError`` rather than ``UnboundLocalError``. (:issue:`16511`) diff --git a/pandas/tests/io/parser/common.py b/pandas/tests/io/parser/common.py index 8c7bb4df864c1..31d815a4bca97 100644 --- a/pandas/tests/io/parser/common.py +++ b/pandas/tests/io/parser/common.py @@ -11,7 +11,6 @@ import pytest import numpy as np -from pandas import read_csv from pandas._libs.lib import Timestamp import pandas as pd @@ -1755,10 +1754,3 @@ def test_skip_bad_lines(self): val = sys.stderr.getvalue() assert 'Skipping line 3' in val assert 'Skipping line 5' in val - - def test_unknown_engine(self): - with tm.ensure_clean() as path: - df = tm.makeDataFrame() - df.to_csv(path) - with tm.assert_raises_regex(ValueError, 'Unknown engine'): - read_csv(path, engine='pyt') diff --git a/pandas/tests/io/test_common.py b/pandas/tests/io/test_common.py index b7d158dd75960..289f86eb2dc53 100644 --- a/pandas/tests/io/test_common.py +++ b/pandas/tests/io/test_common.py @@ -223,3 +223,10 @@ def test_next(self): assert next_line.strip() == line.strip() pytest.raises(StopIteration, next, wrapper) + + def test_unknown_engine(self): + with tm.ensure_clean() as path: + df = tm.makeDataFrame() + df.to_csv(path) + with tm.assert_raises_regex(ValueError, 'Unknown engine'): + read_csv(path, engine='pyt')