Repository: pandas-dev/pandas
Pull request: Fix read parquet import error message
- [x] closes #33313
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry  N/A

`pandas.io.parquet.get_engine()` uses handling of `ImportError`s for flow control to decide which parquet reader engine is used. In doing so, it quashed lower-level error messages that would have been helpful to the user attempting to diagnose the error, replacing it with a misleading error message. 

I refactored the error handling, allowing for these lower-level error messages to be collected and explicitly "bubbled up". Thus fixing the incorrect error message.

No tests added -- this behaviour is not worthy of testing. Presumably not worthy of a whatsnew entry either.

Problem statement:
BUG: Bad error message on read_parquet() when wrong version of pyarrow is installed
- [x] I have checked that this issue has not already been reported.

- [x] I have confirmed this bug exists on the latest version of pandas.

- [x] (optional) I have confirmed this bug exists on the master branch of pandas.

---

#### Code Sample, a copy-pastable example

With `pyarrow<0.13` installed, 
```python
pd.read_parquet('some_file.parquet')
```

#### Problem description

The error message says that `pyarrow` should be installed, even though `pyarrow` is already installed – which makes the error difficult to diagnose and fix, when in fact, the problem is an incompatible _version_ of pyarrow.

```
  File "[...]\lib\site-packages\pandas\io\parquet.py", line 33, in get_engine
    "Unable to find a usable engine; "
ImportError: Unable to find a usable engine; tried using: 'pyarrow', 'fastparquet'.
pyarrow or fastparquet is required for parquet support
```

#### Expected Output

The message should state the real cause of the error, namely an incompatible version of pyarrow. Like:

```
  File "[...]\lib\site-packages\pandas\io\parquet.py", line 40, in get_engine
    return PyArrowImpl()
  File "[...]\lib\site-packages\pandas\io\parquet.py", line 75, in __init__
    "pyarrow", extra="pyarrow is required for parquet support."
  File "[...]\lib\site-packages\pandas\compat\_optional.py", line 109, in import_optional_dependency
    raise ImportError(msg)
ImportError: Pandas requires version '0.13.0' or newer of 'pyarrow' (version '0.11.1' currently installed).
```

This helpful error message does in fact get generated at a lower level in `pandas.compat._optional.import_optional_dependency()`, but then gets swallowed when its client `pandas.io.parquet.get_engine(engine="auto")` raises a new `ImportError` from scratch. 

#### Output of ``pd.show_versions()``

<details>

INSTALLED VERSIONS
------------------
commit           : None
python           : 3.7.7.final.0
python-bits      : 64
OS               : Windows
OS-release       : 10
machine          : AMD64
processor        : Intel64 Family 6 Model 61 Stepping 4, GenuineIntel
byteorder        : little
LC_ALL           : None
LANG             : en_US.UTF-8
LOCALE           : None.None

pandas           : 1.0.3
numpy            : 1.18.1
pytz             : 2019.3
dateutil         : 2.8.1
pip              : 20.0.2
setuptools       : 46.1.3.post20200330
Cython           : None
pytest           : None
hypothesis       : None
sphinx           : None
blosc            : None
feather          : None
xlsxwriter       : None
lxml.etree       : None
html5lib         : None
pymysql          : None
psycopg2         : None
jinja2           : None
IPython          : None
pandas_datareader: None
bs4              : None
bottleneck       : None
fastparquet      : None
gcsfs            : None
lxml.etree       : None
matplotlib       : None
numexpr          : None
odfpy            : None
openpyxl         : 3.0.3
pandas_gbq       : None
pyarrow          : 0.11.1
pytables         : None
pytest           : None
pyxlsb           : None
s3fs             : None
scipy            : 1.4.1
sqlalchemy       : None
tables           : None
tabulate         : None
xarray           : None
xlrd             : None
xlwt             : None
xlsxwriter       : None
numba            : None

</details>

Hints from issue discussion:
@jfcorbett thanks for the report! That error message could indeed be improved.

Contributions welcome!
take

Commit message: Merge remote-tracking branch 'upstream/master' into fix-read-parquet-import-error-message

Patch to review:
diff --git a/pandas/io/parquet.py b/pandas/io/parquet.py
index 46320355512d1..12b3ff56683f0 100644
--- a/pandas/io/parquet.py
+++ b/pandas/io/parquet.py
@@ -18,20 +18,23 @@ def get_engine(engine: str) -> "BaseImpl":
 
     if engine == "auto":
         # try engines in this order
-        try:
-            return PyArrowImpl()
-        except ImportError:
-            pass
+        engine_impls = [PyArrowImpl, FastParquetImpl]
 
-        try:
-            return FastParquetImpl()
-        except ImportError:
-            pass
+        error_msgs = []
+        for eimpl in engine_impls:
+            try:
+                return eimpl()
+            except ImportError as ie:
+                error_msgs.append(ie.msg)
 
         raise ImportError(
             "Unable to find a usable engine; "
             "tried using: 'pyarrow', 'fastparquet'.\n"
-            "pyarrow or fastparquet is required for parquet support"
+            "A suitable version of "
+            "pyarrow or fastparquet is required for parquet "
+            "support. \n"
+            "Trying to import the above resulted in these errors: \n"
+            + "\n".join(error_msgs)
         )
 
     if engine == "pyarrow":
@@ -105,9 +108,7 @@ def write(
                 **kwargs,
             )
         else:
-            self.api.parquet.write_table(
-                table, path, compression=compression, **kwargs,
-            )
+            self.api.parquet.write_table(table, path, compression=compression, **kwargs)
 
     def read(self, path, columns=None, **kwargs):
         path, _, _, should_close = get_filepath_or_buffer(path)


Code review comments:
diff_hunk: @@ -18,20 +18,23 @@ def get_engine(engine: str) -> "BaseImpl":
 
     if engine == "auto":
         # try engines in this order
-        try:
-            return PyArrowImpl()
-        except ImportError:
-            pass
+        engine_impls = [PyArrowImpl, FastParquetImpl]
 
-        try:
-            return FastParquetImpl()
-        except ImportError:
-            pass
+        error_msgs = []
+        for eimpl in engine_impls:
path: pandas/io/parquet.py
text: @user1:
May be use `engine_classes` and `engine_class`? Personally I find these abbreviations a bit confusing and distracting.

diff_hunk: @@ -18,20 +18,23 @@ def get_engine(engine: str) -> "BaseImpl":
 
     if engine == "auto":
         # try engines in this order
-        try:
-            return PyArrowImpl()
-        except ImportError:
-            pass
+        engine_impls = [PyArrowImpl, FastParquetImpl]
 
-        try:
-            return FastParquetImpl()
-        except ImportError:
-            pass
+        error_msgs = []
+        for eimpl in engine_impls:
+            try:
+                return eimpl()
+            except ImportError as ie:
+                error_msgs.append(ie.msg)
 
         raise ImportError(
             "Unable to find a usable engine; "
             "tried using: 'pyarrow', 'fastparquet'.\n"
-            "pyarrow or fastparquet is required for parquet support"
+            "A suitable version of "
+            "pyarrow or fastparquet is required for parquet "
+            "support. \n"
+            "Trying to import the above resulted in these errors: \n"
path: pandas/io/parquet.py
text: @user1:
```suggestion
            "Trying to import the above resulted in these errors:\n"
```

diff_hunk: @@ -18,20 +18,23 @@ def get_engine(engine: str) -> "BaseImpl":
 
     if engine == "auto":
         # try engines in this order
-        try:
-            return PyArrowImpl()
-        except ImportError:
-            pass
+        engine_impls = [PyArrowImpl, FastParquetImpl]
 
-        try:
-            return FastParquetImpl()
-        except ImportError:
-            pass
+        error_msgs = []
+        for eimpl in engine_impls:
+            try:
+                return eimpl()
+            except ImportError as ie:
+                error_msgs.append(ie.msg)
 
         raise ImportError(
             "Unable to find a usable engine; "
             "tried using: 'pyarrow', 'fastparquet'.\n"
-            "pyarrow or fastparquet is required for parquet support"
+            "A suitable version of "
+            "pyarrow or fastparquet is required for parquet "
+            "support. \n"
+            "Trying to import the above resulted in these errors: \n"
+            + "\n".join(error_msgs)
path: pandas/io/parquet.py
text: @user1:
Can you take the ```join``` out side the string? and then use just f-strings, to look somewhat like this:

```python
joined = "\n".join(error_msgs)
raise ImportError("
            "Unable to find a usable engine; "
            "tried using: 'pyarrow', 'fastparquet'.\n"
            "A suitable version of pyarrow or fastparquet is required for parquet support. \n"
            "Trying to import the above resulted in these errors: \n"
            f"{joined}"
)
```

diff_hunk: @@ -18,20 +18,23 @@ def get_engine(engine: str) -> "BaseImpl":
 
     if engine == "auto":
         # try engines in this order
-        try:
-            return PyArrowImpl()
-        except ImportError:
-            pass
+        engine_impls = [PyArrowImpl, FastParquetImpl]
 
-        try:
-            return FastParquetImpl()
-        except ImportError:
-            pass
+        error_msgs = []
+        for eimpl in engine_impls:
+            try:
+                return eimpl()
+            except ImportError as ie:
+                error_msgs.append(ie.msg)
path: pandas/io/parquet.py
text: @user1:
mypy is complaining because of this, not sure is `str(ie)` would be the correct way.

@author:
Yup, saw that in CI, now fixed!

diff_hunk: @@ -18,20 +18,23 @@ def get_engine(engine: str) -> "BaseImpl":
 
     if engine == "auto":
         # try engines in this order
-        try:
-            return PyArrowImpl()
-        except ImportError:
-            pass
+        engine_impls = [PyArrowImpl, FastParquetImpl]
 
-        try:
-            return FastParquetImpl()
-        except ImportError:
-            pass
+        error_msgs = []
+        for eimpl in engine_impls:
+            try:
+                return eimpl()
+            except ImportError as ie:
+                error_msgs.append(ie.msg)
 
         raise ImportError(
             "Unable to find a usable engine; "
             "tried using: 'pyarrow', 'fastparquet'.\n"
-            "pyarrow or fastparquet is required for parquet support"
+            "A suitable version of "
+            "pyarrow or fastparquet is required for parquet "
+            "support. \n"
path: pandas/io/parquet.py
text: @user1:
```suggestion
            "support.\n"
```

diff_hunk: @@ -18,20 +18,23 @@ def get_engine(engine: str) -> "BaseImpl":
 
     if engine == "auto":
         # try engines in this order
-        try:
-            return PyArrowImpl()
-        except ImportError:
-            pass
+        engine_impls = [PyArrowImpl, FastParquetImpl]
 
-        try:
-            return FastParquetImpl()
-        except ImportError:
-            pass
+        error_msgs = []
+        for eimpl in engine_impls:
+            try:
+                return eimpl()
+            except ImportError as ie:
path: pandas/io/parquet.py
text: @user1:
I think we always use `as e` or as `as err`.

@user2:
I think ```as err``` as we are trying to avoid one letter variable (*I think*)