Repository: IAMconsortium/pyam

Issue:
`timeseries()` on empty `IamDataFrame` fails very loudly
# Description of the issue

When calling `timeseries()` on an empty `IamDataFrame`, the internally used `pandas.DataFrame.pivot_table()` fails with a long, non-intuitive error message (see below). This might be quite intimidating to non-expert users.

## Improved behaviour

Checking whether the `data` table is empty before doing the pivot operation is straightforward. 

Two questions on implementation and expected behaviour, though:

1) Should `pyam` only write a message to the log file (as is done for `filter()`, see [here](https://github.com/IAMconsortium/pyam/blob/103b60f8f2f46fddae4d0325cbc2651cb098030a/pyam/core.py#L1001), or should a (nicer) error be raised?

2) If only a warning is written to the log, should the returned object be `None` or an empty `pandas.DataFrame` (but with the correct index dimensions).

# Error message

```
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-48-36a29f3c3d4a> in <module>
----> 1 df.filter(variable='foo').timeseries()

~/Github/pyam/pyam/core.py in timeseries(self, iamc_index)
    382         df = (
    383             self.data
--> 384             .pivot_table(index=index, columns=self.time_col)
    385             .value  # column name
    386             .rename_axis(None, axis=1)

~/anaconda3/lib/python3.7/site-packages/pandas/core/generic.py in __getattr__(self, name)
   5177             if self._info_axis._can_hold_identifiers_and_holds_name(name):
   5178                 return self[name]
-> 5179             return object.__getattribute__(self, name)
   5180 
   5181     def __setattr__(self, name, value):

AttributeError: 'DataFrame' object has no attribute 'value'
```

Discussion:
znicholls: Oh nice then keep it as is, I was just doing what seemed to  read more
easily to me but consistency with the wider community is a better choice

On Sat, 30 Nov 2019 at 6:07 pm, Daniel Huppmann <notifications@github.com>
wrote:

> *@danielhuppmann* commented on this pull request.
> ------------------------------
>
> In pyam/core.py
> <https://github.com/IAMconsortium/pyam/pull/297#discussion_r352270626>:
>
> > @@ -148,6 +148,11 @@ def tail(self, *args, **kwargs):
>          """Identical to pd.DataFrame.tail() operating on data"""
>          return self.data.tail(*args, **kwargs)
>
> +    @property
> +    def empty(self):
> +        """Indicator whether ``IamDataFrame`` is empty"""
>
> I was using the pandas docstring as a template, see
> https://pandas.pydata.org/pandas-docs/stable/reference/api/pandas.DataFrame.empty.html
>
> —
> You are receiving this because your review was requested.
> Reply to this email directly, view it on GitHub
> <https://github.com/IAMconsortium/pyam/pull/297?email_source=notifications&email_token=AFUH5GZ6BLHMECEZHCRL6STQWIGJHA5CNFSM4JRZ44AKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCNOUQBA#discussion_r352270626>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AFUH5GYTR357ETI4TKEYSOTQWIGJHANCNFSM4JRZ44AA>
> .
>

danielhuppmann: thanks @znicholls for the review!
danielhuppmann: Merging as approved by @znicholls

Patch:
diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md
index c868d77..c43a18f 100644
--- a/RELEASE_NOTES.md
+++ b/RELEASE_NOTES.md
@@ -1,6 +1,7 @@
 
 # Next Release
 
+- [#297](https://github.com/IAMconsortium/pyam/pull/297) Add `empty` attribute, better error for `timeseries()` on empty dataframe 
 - [#292](https://github.com/IAMconsortium/pyam/pull/292) Add warning message if `data` is empty at initialization (after formatting)
 - [#288](https://github.com/IAMconsortium/pyam/pull/288) Put `pyam` logger in its own namespace (see [here](https://docs.python-guide.org/writing/logging/#logging-in-a-library>))
 
diff --git a/pyam/core.py b/pyam/core.py
index d5787f1..03837dd 100644
--- a/pyam/core.py
+++ b/pyam/core.py
@@ -148,6 +148,11 @@ class IamDataFrame(object):
         """Identical to pd.DataFrame.tail() operating on data"""
         return self.data.tail(*args, **kwargs)
 
+    @property
+    def empty(self):
+        """Indicator whether ``IamDataFrame`` is empty"""
+        return self.data.empty
+
     def models(self):
         """Get a list of models"""
         return pd.Series(self.meta.index.levels[0])
@@ -373,14 +378,24 @@ class IamDataFrame(object):
         return list(cols)
 
     def timeseries(self, iamc_index=False):
-        """Returns a pd.DataFrame in wide format (years or timedate as columns)
+        """Returns a pd.DataFrame in wide format (years or datetime as columns)
 
         Parameters
         ----------
         iamc_index: bool, default False
             if True, use `['model', 'scenario', 'region', 'variable', 'unit']`;
             else, use all `data` columns
+
+        Raises
+        ------
+        ValueError
+            ``IamDataFrame`` is empty
+        ValueError
+            reducing to IAMC-index yields an index with duplicates
         """
+        if self.empty:
+            raise ValueError('this `IamDataFrame` is empty')
+
         index = IAMC_IDX if iamc_index else IAMC_IDX + self.extra_cols
         df = (
             self.data


Test patch:
diff --git a/tests/test_core.py b/tests/test_core.py
index 58fb1ef..ae3965a 100644
--- a/tests/test_core.py
+++ b/tests/test_core.py
@@ -152,6 +152,11 @@ def test_init_empty_message(test_pd_df, caplog):
     assert caplog.records[message_idx].levelno == logging.WARNING
 
 
+def test_empty_attribute(test_df_year):
+    assert not test_df_year.empty
+    assert test_df_year.filter(model='foo').empty
+
+
 def test_to_excel(test_df):
     fname = 'foo_testing.xlsx'
     test_df.to_excel(fname)
@@ -491,6 +496,11 @@ def test_timeseries(test_df):
     npt.assert_array_equal(obs, exp)
 
 
+def test_timeseries_raises(test_df_year):
+    _df = test_df_year.filter(model='foo')
+    pytest.raises(ValueError, _df.timeseries)
+
+
 def test_read_pandas():
     df = IamDataFrame(os.path.join(TEST_DATA_DIR, 'testing_data_2.csv'))
     assert list(df.variables()) == ['Primary Energy']
diff --git a/tests/test_tutorials.py b/tests/test_tutorials.py
index 8f4560d..4b4f5c6 100644
--- a/tests/test_tutorials.py
+++ b/tests/test_tutorials.py
@@ -28,7 +28,7 @@ tut_path = os.path.join(here, '..', 'doc', 'source', 'tutorials')
 # https://blog.thedataincubator.com/2016/06/testing-jupyter-notebooks/
 
 
-def _notebook_run(path, kernel=None, capsys=None):
+def _notebook_run(path, kernel=None, timeout=60, capsys=None):
     """Execute a notebook via nbconvert and collect output.
     :returns (parsed nb object, execution errors)
     """
@@ -39,7 +39,7 @@ def _notebook_run(path, kernel=None, capsys=None):
     fname = os.path.join(here, 'test.ipynb')
     args = [
         "jupyter", "nbconvert", "--to", "notebook", "--execute",
-        "--ExecutePreprocessor.timeout=60",
+        "--ExecutePreprocessor.timeout={}".format(timeout),
         "--ExecutePreprocessor.kernel_name={}".format(kernel),
         "--output", fname, path]
     subprocess.check_call(args)
@@ -87,7 +87,7 @@ def test_pyam_logo():
 @pytest.mark.skipif(not pandoc_installed, reason=pandoc_reason)
 def test_iiasa_dbs():
     fname = os.path.join(tut_path, 'iiasa_dbs.ipynb')
-    nb, errors = _notebook_run(fname)
+    nb, errors = _notebook_run(fname, timeout=600)
     assert errors == []