From c4bbc69250d299dea9c793ea08ea30b85fecb0c1 Mon Sep 17 00:00:00 2001 From: DanielEvans Date: Wed, 5 Jun 2019 14:51:31 +0100 Subject: [PATCH 01/13] Modify error message for missing dependencies to report original error message --- pandas/__init__.py | 8 +++++--- pandas/tests/test_base.py | 27 +++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/pandas/__init__.py b/pandas/__init__.py index 4c494b4a62e39..11ea3047bb62a 100644 --- a/pandas/__init__.py +++ b/pandas/__init__.py @@ -10,11 +10,13 @@ try: __import__(dependency) except ImportError as e: - missing_dependencies.append(dependency) + missing_dependencies.append((dependency, e)) if missing_dependencies: - raise ImportError( - "Missing required dependencies {0}".format(missing_dependencies)) + msg = "Unable to import required dependencies:" + for dependency, e in missing_dependencies: + msg += "\n{0}: {1}".format(dependency, str(e)) + raise ImportError(msg) del hard_dependencies, dependency, missing_dependencies # numpy compat diff --git a/pandas/tests/test_base.py b/pandas/tests/test_base.py index 3b4f85e680f6e..f8319999682e8 100644 --- a/pandas/tests/test_base.py +++ b/pandas/tests/test_base.py @@ -1,7 +1,9 @@ from datetime import datetime, timedelta +from importlib import reload from io import StringIO import re import sys +from unittest.mock import patch import numpy as np import pytest @@ -1341,3 +1343,28 @@ def test_to_numpy_dtype(as_series): expected = np.array(['2000-01-01T05', '2001-01-01T05'], dtype='M8[ns]') tm.assert_numpy_array_equal(result, expected) + + +@patch("builtins.__import__") +def test_missing_required_dependency(mock_import): + def mock_import_fail(name, *args, **kwargs): + if name == "numpy": + raise ImportError("cannot import name numpy") + elif name == "pytz": + raise ImportError("cannot import name some_dependency") + elif name == "dateutil": + raise ImportError("cannot import name some_other_dependency") + else: + return __import__(name, *args, **kwargs) + + mock_import.side_effect = mock_import_fail + + expected_msg = ( + "Unable to import required dependencies:" + "\nnumpy: cannot import name numpy" + "\npytz: cannot import name some_dependency" + "\ndateutil: cannot import name some_other_dependency" + ) + + with pytest.raises(ImportError, match=expected_msg): + reload(pd) From 70da2333144fc083183641e54068f566985099e5 Mon Sep 17 00:00:00 2001 From: DanielEvans Date: Wed, 5 Jun 2019 15:00:36 +0100 Subject: [PATCH 02/13] Update whatsnew --- doc/source/whatsnew/v0.25.0.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/source/whatsnew/v0.25.0.rst b/doc/source/whatsnew/v0.25.0.rst index 4018418294963..8fd9f07442810 100644 --- a/doc/source/whatsnew/v0.25.0.rst +++ b/doc/source/whatsnew/v0.25.0.rst @@ -82,7 +82,7 @@ Other Enhancements - :meth:`DataFrame.query` and :meth:`DataFrame.eval` now supports quoting column names with backticks to refer to names with spaces (:issue:`6508`) - :func:`merge_asof` now gives a more clear error message when merge keys are categoricals that are not equal (:issue:`26136`) - :meth:`pandas.core.window.Rolling` supports exponential (or Poisson) window type (:issue:`21303`) -- +- Error message for missing required imports now includes the original ImportError's text (:issue:`23868`) .. _whatsnew_0250.api_breaking: From ab47d9904af44d1fe712042a1be0aff4fd0e9361 Mon Sep 17 00:00:00 2001 From: DanielEvans Date: Thu, 6 Jun 2019 08:40:16 +0100 Subject: [PATCH 03/13] Fix for recursion in test --- pandas/tests/test_base.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/pandas/tests/test_base.py b/pandas/tests/test_base.py index f8319999682e8..3fec5abe426f7 100644 --- a/pandas/tests/test_base.py +++ b/pandas/tests/test_base.py @@ -1347,6 +1347,8 @@ def test_to_numpy_dtype(as_series): @patch("builtins.__import__") def test_missing_required_dependency(mock_import): + original_import = __import__ + def mock_import_fail(name, *args, **kwargs): if name == "numpy": raise ImportError("cannot import name numpy") @@ -1355,9 +1357,7 @@ def mock_import_fail(name, *args, **kwargs): elif name == "dateutil": raise ImportError("cannot import name some_other_dependency") else: - return __import__(name, *args, **kwargs) - - mock_import.side_effect = mock_import_fail + return original_import(name, *args, **kwargs) expected_msg = ( "Unable to import required dependencies:" @@ -1366,5 +1366,7 @@ def mock_import_fail(name, *args, **kwargs): "\ndateutil: cannot import name some_other_dependency" ) - with pytest.raises(ImportError, match=expected_msg): - reload(pd) + with patch("builtins.__import__") as mock_import: + mock_import.side_effect = mock_import_fail + with pytest.raises(ImportError, match=expected_msg): + reload(pd) From 9556fca39112a5d952f8045f6708ac160761c931 Mon Sep 17 00:00:00 2001 From: DanielEvans Date: Thu, 6 Jun 2019 09:14:51 +0100 Subject: [PATCH 04/13] Remove duplicate test patching --- pandas/tests/test_base.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pandas/tests/test_base.py b/pandas/tests/test_base.py index 3fec5abe426f7..4f6591da89569 100644 --- a/pandas/tests/test_base.py +++ b/pandas/tests/test_base.py @@ -1345,8 +1345,7 @@ def test_to_numpy_dtype(as_series): tm.assert_numpy_array_equal(result, expected) -@patch("builtins.__import__") -def test_missing_required_dependency(mock_import): +def test_missing_required_dependency(): original_import = __import__ def mock_import_fail(name, *args, **kwargs): From 6eecf8b151e170da267b1513ea01cafadc0d7359 Mon Sep 17 00:00:00 2001 From: DanielEvans Date: Thu, 6 Jun 2019 09:52:27 +0100 Subject: [PATCH 05/13] Try re-reloading Pandas to ensure good state after test --- pandas/tests/test_base.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pandas/tests/test_base.py b/pandas/tests/test_base.py index 4f6591da89569..88b615f61fd46 100644 --- a/pandas/tests/test_base.py +++ b/pandas/tests/test_base.py @@ -1369,3 +1369,6 @@ def mock_import_fail(name, *args, **kwargs): mock_import.side_effect = mock_import_fail with pytest.raises(ImportError, match=expected_msg): reload(pd) + + # Final reload to ensure normal state of pandas + reload(pd) From 7e0992830fbe4425995d749f3515032bed61a09a Mon Sep 17 00:00:00 2001 From: DanielEvans Date: Thu, 6 Jun 2019 10:27:58 +0100 Subject: [PATCH 06/13] Alter definitions of temp variables in __init__ so they can be explicitly deleted --- pandas/__init__.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pandas/__init__.py b/pandas/__init__.py index 11ea3047bb62a..da0b0807f260d 100644 --- a/pandas/__init__.py +++ b/pandas/__init__.py @@ -5,6 +5,8 @@ # Let users know if they're missing any of our hard dependencies hard_dependencies = ("numpy", "pytz", "dateutil") missing_dependencies = [] +msg = None +e = None for dependency in hard_dependencies: try: @@ -17,7 +19,7 @@ for dependency, e in missing_dependencies: msg += "\n{0}: {1}".format(dependency, str(e)) raise ImportError(msg) -del hard_dependencies, dependency, missing_dependencies +del hard_dependencies, dependency, missing_dependencies, e, msg # numpy compat from pandas.compat.numpy import ( From fa8502559f41edd3633b1acadd7de180d6fc70fa Mon Sep 17 00:00:00 2001 From: DanielEvans Date: Thu, 6 Jun 2019 11:04:59 +0100 Subject: [PATCH 07/13] Use monkeypatch, not unittest --- pandas/tests/test_base.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pandas/tests/test_base.py b/pandas/tests/test_base.py index 88b615f61fd46..dbb673e4df52a 100644 --- a/pandas/tests/test_base.py +++ b/pandas/tests/test_base.py @@ -1,9 +1,9 @@ +import builtins from datetime import datetime, timedelta from importlib import reload from io import StringIO import re import sys -from unittest.mock import patch import numpy as np import pytest @@ -1345,7 +1345,7 @@ def test_to_numpy_dtype(as_series): tm.assert_numpy_array_equal(result, expected) -def test_missing_required_dependency(): +def test_missing_required_dependency(monkeypatch): original_import = __import__ def mock_import_fail(name, *args, **kwargs): @@ -1365,8 +1365,8 @@ def mock_import_fail(name, *args, **kwargs): "\ndateutil: cannot import name some_other_dependency" ) - with patch("builtins.__import__") as mock_import: - mock_import.side_effect = mock_import_fail + with monkeypatch.context() as m: + m.setattr(builtins, "__import__", mock_import_fail) with pytest.raises(ImportError, match=expected_msg): reload(pd) From 081327ff741aa2313feeeb24888960f47d0546f0 Mon Sep 17 00:00:00 2001 From: DanielEvans Date: Thu, 6 Jun 2019 11:10:37 +0100 Subject: [PATCH 08/13] Changes to satisfy mypy --- pandas/__init__.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pandas/__init__.py b/pandas/__init__.py index da0b0807f260d..05085d498f62a 100644 --- a/pandas/__init__.py +++ b/pandas/__init__.py @@ -5,8 +5,6 @@ # Let users know if they're missing any of our hard dependencies hard_dependencies = ("numpy", "pytz", "dateutil") missing_dependencies = [] -msg = None -e = None for dependency in hard_dependencies: try: @@ -14,12 +12,14 @@ except ImportError as e: missing_dependencies.append((dependency, e)) +msg = None +ex = None if missing_dependencies: msg = "Unable to import required dependencies:" - for dependency, e in missing_dependencies: - msg += "\n{0}: {1}".format(dependency, str(e)) + for dependency, ex in missing_dependencies: + msg += "\n{0}: {1}".format(dependency, str(ex)) raise ImportError(msg) -del hard_dependencies, dependency, missing_dependencies, e, msg +del hard_dependencies, dependency, missing_dependencies, ex, msg # numpy compat from pandas.compat.numpy import ( From 0e3038f15cd13955f577b3ef3f9ee25943841ecb Mon Sep 17 00:00:00 2001 From: DanielEvans Date: Thu, 6 Jun 2019 11:23:26 +0100 Subject: [PATCH 09/13] Move import test to test_downstream --- pandas/tests/test_base.py | 31 ------------------------------- pandas/tests/test_downstream.py | 30 ++++++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 31 deletions(-) diff --git a/pandas/tests/test_base.py b/pandas/tests/test_base.py index dbb673e4df52a..3b4f85e680f6e 100644 --- a/pandas/tests/test_base.py +++ b/pandas/tests/test_base.py @@ -1,6 +1,4 @@ -import builtins from datetime import datetime, timedelta -from importlib import reload from io import StringIO import re import sys @@ -1343,32 +1341,3 @@ def test_to_numpy_dtype(as_series): expected = np.array(['2000-01-01T05', '2001-01-01T05'], dtype='M8[ns]') tm.assert_numpy_array_equal(result, expected) - - -def test_missing_required_dependency(monkeypatch): - original_import = __import__ - - def mock_import_fail(name, *args, **kwargs): - if name == "numpy": - raise ImportError("cannot import name numpy") - elif name == "pytz": - raise ImportError("cannot import name some_dependency") - elif name == "dateutil": - raise ImportError("cannot import name some_other_dependency") - else: - return original_import(name, *args, **kwargs) - - expected_msg = ( - "Unable to import required dependencies:" - "\nnumpy: cannot import name numpy" - "\npytz: cannot import name some_dependency" - "\ndateutil: cannot import name some_other_dependency" - ) - - with monkeypatch.context() as m: - m.setattr(builtins, "__import__", mock_import_fail) - with pytest.raises(ImportError, match=expected_msg): - reload(pd) - - # Final reload to ensure normal state of pandas - reload(pd) diff --git a/pandas/tests/test_downstream.py b/pandas/tests/test_downstream.py index c41c3abcab4af..70d00ba17beba 100644 --- a/pandas/tests/test_downstream.py +++ b/pandas/tests/test_downstream.py @@ -1,6 +1,7 @@ """ Testing that we work in the downstream packages """ +import builtins import importlib import subprocess import sys @@ -131,3 +132,32 @@ def test_pyarrow(df): table = pyarrow.Table.from_pandas(df) result = table.to_pandas() tm.assert_frame_equal(result, df) + + +def test_missing_required_dependency(monkeypatch): + original_import = __import__ + + def mock_import_fail(name, *args, **kwargs): + if name == "numpy": + raise ImportError("cannot import name numpy") + elif name == "pytz": + raise ImportError("cannot import name some_dependency") + elif name == "dateutil": + raise ImportError("cannot import name some_other_dependency") + else: + return original_import(name, *args, **kwargs) + + expected_msg = ( + "Unable to import required dependencies:" + "\nnumpy: cannot import name numpy" + "\npytz: cannot import name some_dependency" + "\ndateutil: cannot import name some_other_dependency" + ) + + import pandas as pd + + with monkeypatch.context() as m: + m.setattr(builtins, "__import__", mock_import_fail) + with pytest.raises(ImportError, match=expected_msg): + importlib.reload(pd) + From c07f751e739868566b01d7c727b5fafce0a2f9aa Mon Sep 17 00:00:00 2001 From: DanielEvans Date: Thu, 6 Jun 2019 11:24:48 +0100 Subject: [PATCH 10/13] Linting --- pandas/tests/test_downstream.py | 1 - 1 file changed, 1 deletion(-) diff --git a/pandas/tests/test_downstream.py b/pandas/tests/test_downstream.py index 70d00ba17beba..aa2e839843998 100644 --- a/pandas/tests/test_downstream.py +++ b/pandas/tests/test_downstream.py @@ -160,4 +160,3 @@ def mock_import_fail(name, *args, **kwargs): m.setattr(builtins, "__import__", mock_import_fail) with pytest.raises(ImportError, match=expected_msg): importlib.reload(pd) - From 6216ebfbf1e8aaab0fac955d0af4fcd36009703b Mon Sep 17 00:00:00 2001 From: DanielEvans Date: Thu, 6 Jun 2019 15:37:09 +0100 Subject: [PATCH 11/13] Simplify missing dependency error formatting --- pandas/__init__.py | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/pandas/__init__.py b/pandas/__init__.py index 05085d498f62a..9b0a80a5c8e20 100644 --- a/pandas/__init__.py +++ b/pandas/__init__.py @@ -10,16 +10,11 @@ try: __import__(dependency) except ImportError as e: - missing_dependencies.append((dependency, e)) + missing_dependencies.append("{0}: {1}".format(dependency, str(e))) -msg = None -ex = None if missing_dependencies: - msg = "Unable to import required dependencies:" - for dependency, ex in missing_dependencies: - msg += "\n{0}: {1}".format(dependency, str(ex)) - raise ImportError(msg) -del hard_dependencies, dependency, missing_dependencies, ex, msg + raise ImportError("Unable to import required dependencies:" + "\n".join(missing_dependencies)) +del hard_dependencies, dependency, missing_dependencies # numpy compat from pandas.compat.numpy import ( From 3b00019f53e0acfa78842586a1e07b24752d2a56 Mon Sep 17 00:00:00 2001 From: DanielEvans Date: Fri, 7 Jun 2019 08:49:32 +0100 Subject: [PATCH 12/13] Correct error message formatting, minor change to changelog wording --- doc/source/whatsnew/v0.25.0.rst | 2 +- pandas/__init__.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/source/whatsnew/v0.25.0.rst b/doc/source/whatsnew/v0.25.0.rst index 8fd9f07442810..702ae9467070c 100644 --- a/doc/source/whatsnew/v0.25.0.rst +++ b/doc/source/whatsnew/v0.25.0.rst @@ -82,7 +82,7 @@ Other Enhancements - :meth:`DataFrame.query` and :meth:`DataFrame.eval` now supports quoting column names with backticks to refer to names with spaces (:issue:`6508`) - :func:`merge_asof` now gives a more clear error message when merge keys are categoricals that are not equal (:issue:`26136`) - :meth:`pandas.core.window.Rolling` supports exponential (or Poisson) window type (:issue:`21303`) -- Error message for missing required imports now includes the original ImportError's text (:issue:`23868`) +- Error message for missing required imports now includes the original import error's text (:issue:`23868`) .. _whatsnew_0250.api_breaking: diff --git a/pandas/__init__.py b/pandas/__init__.py index 9b0a80a5c8e20..a2fa14be83998 100644 --- a/pandas/__init__.py +++ b/pandas/__init__.py @@ -13,7 +13,7 @@ missing_dependencies.append("{0}: {1}".format(dependency, str(e))) if missing_dependencies: - raise ImportError("Unable to import required dependencies:" + "\n".join(missing_dependencies)) + raise ImportError("Unable to import required dependencies:\n" + "\n".join(missing_dependencies)) del hard_dependencies, dependency, missing_dependencies # numpy compat From 0fff1fb448e0807084a140a9f35ba65fb20464ea Mon Sep 17 00:00:00 2001 From: DanielEvans Date: Mon, 10 Jun 2019 08:50:17 +0100 Subject: [PATCH 13/13] Add issue number to new test --- pandas/tests/test_downstream.py | 1 + 1 file changed, 1 insertion(+) diff --git a/pandas/tests/test_downstream.py b/pandas/tests/test_downstream.py index aa2e839843998..14d3ee5ac4fe2 100644 --- a/pandas/tests/test_downstream.py +++ b/pandas/tests/test_downstream.py @@ -135,6 +135,7 @@ def test_pyarrow(df): def test_missing_required_dependency(monkeypatch): + # GH 23868 original_import = __import__ def mock_import_fail(name, *args, **kwargs):