Repository: IAMconsortium/pyam
Pull request #114 (2018-10-19 16:22:11)

extend `append()` function to allow adding data
# Please confirm that this PR has done the following:

- [x] Tests Added
- [x] Documentation Added
- [x] Description in RELEASE_NOTES.md Added

# Description of PR

This PR extends the `append()` function to allow adding data for existing scenarios in an IamDataFrame. Until now, the function only accepted data for model-scenarios that were not in the index of the IamDataFrame to which data was appended.

In practice, I often wanted to add data to an existing scenario, like pull out GDP and population data, compute a timeseries GDP per capita, and append it. I the implemented this by 
```
df.data = df.data.append(other.data)
```
The drawback here is that this creates duplicated timeseries data in the IamDataFrame if one executes this line more than once.

Now, the function accepts new timeseries data as long as there is no overlap with existing data.

danielhuppmann: @gidden, an API question for you: assume I have two IamDataFrames with the same model/scenario and conflicting meta column entries, like

``data.meta``

|    |    |exclude| col1 |
|--|---|------|------|
|m | s | False | a       |

``other.meta``

|    |    |exclude| col1 | col2 |
|--|---|------|------|------|
|m | s | False | b       | 1       |

The way I implemented it now, the values in `meta` are taken from `data.meta` wherever possible, and only columns that don't exist in `data.meta` are appended from `other.meta`.

``data.append(other).meta``

|    |    |exclude| col1 | col2 |
|--|---|------|------|------|
|m | s | False | a       | 1       |

That means the `other.meta.col1 = b` is simply ignored in `append()`. Not sure if that is the ideal behaviour here - any ideas?
danielhuppmann: @gidden, what is the `pandas` version on CI? Seems that it doesn't know `pd.append(sort=False)`...
gidden: It looks like sort is "New in version 0.23.0.". The question now is whether we want to support backwards pandas dependency. I would suggest one of two options:

- check pandas version here with `_kwargs = dict(sort=True) if pands.__version__[1] >= 23 else {}`
- check pandas version on import with 
```
import pandas
if pandas.__version__[1] < 23:
    raise ImportError('Pandas version must be >= 0.23.0')
```

We can discuss which is best.
gidden: It looks like sort is "New in version 0.23.0.". The question now is whether we want to support backwards pandas dependency. I would suggest one of two options:

- check pandas version here with `_kwargs = dict(sort=True) if pands.__version__[1] >= 23 else {}`
- check pandas version on import with 
```
import pandas
if pandas.__version__[1] < 23:
    raise ImportError('Pandas version must be >= 0.23.0')
```

We can discuss which is best.
gidden: It looks like sort is "New in version 0.23.0.". The question now is whether we want to support backwards pandas dependency. I would suggest one of two options:

- check pandas version here with `_kwargs = dict(sort=True) if pands.__version__[1] >= 23 else {}`
- check pandas version on import with 
```
import pandas
if pandas.__version__[1] < 23:
    raise ImportError('Pandas version must be >= 0.23.0')
```

We can discuss which is best.
gidden: It looks like sort is "New in version 0.23.0.". The question now is whether we want to support backwards pandas dependency. I would suggest one of two options:

- check pandas version here with `_kwargs = dict(sort=True) if pands.__version__[1] >= 23 else {}`
- check pandas version on import with 
```
import pandas
if pandas.__version__[1] < 23:
    raise ImportError('Pandas version must be >= 0.23.0')
```

We can discuss which is best.
gidden: It looks like sort is "New in version 0.23.0.". The question now is whether we want to support backwards pandas dependency. I would suggest one of two options:

- check pandas version here with `_kwargs = dict(sort=True) if pands.__version__[1] >= 23 else {}`
- check pandas version on import with 
```
import pandas
if pandas.__version__[1] < 23:
    raise ImportError('Pandas version must be >= 0.23.0')
```

We can discuss which is best.
gidden: It looks like sort is "New in version 0.23.0.". The question now is whether we want to support backwards pandas dependency. I would suggest one of two options:

- check pandas version here with `_kwargs = dict(sort=True) if pands.__version__[1] >= 23 else {}`
- check pandas version on import with 
```
import pandas
if pandas.__version__[1] < 23:
    raise ImportError('Pandas version must be >= 0.23.0')
```

We can discuss which is best.
danielhuppmann: GitHub doesn't want to run the unit tests
gidden: Hmm...
gidden: try `git commit --amend && git push -f origin ...`
danielhuppmann: seems that the GitHub PR page is lagging behind the GitHub fork page... last commit on the branch there is https://github.com/danielhuppmann/pyam/commit/a23ae3916f0c30d1571e5b6d4eb82155f8aca1ea, but this isn't updated on this page
danielhuppmann: updated now, but GitHub seems to have forgotten that it wants to run some tests...
gidden: looking good, just a few suggested changes

Patch:
diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md
index 534e4fc..d22198b 100644
--- a/RELEASE_NOTES.md
+++ b/RELEASE_NOTES.md
@@ -1,6 +1,7 @@
 
 # Next Release
 
+- (#114)[https://github.com/IAMconsortium/pyam/pull/114] extends `append()` such that data can be added to existing scenarios
 - (#111)[https://github.com/IAMconsortium/pyam/pull/111] extends `set_meta()` such that it requires a name (not None)
 - (#109)[https://github.com/IAMconsortium/pyam/pull/109] add ability to fill between and add data ranges in `line_plot()`
 - (#104)[https://github.com/IAMconsortium/pyam/pull/104] fixes a bug with timeseries utils functions, ensures that years are cast as integers
diff --git a/pyam/core.py b/pyam/core.py
index 29be323..1572a04 100644
--- a/pyam/core.py
+++ b/pyam/core.py
@@ -147,28 +147,67 @@ class IamDataFrame(object):
         else:
             return pd.Series(self.data.variable.unique(), name='variable')
 
-    def append(self, other, inplace=False, **kwargs):
-        """Import or read timeseries data and append to IamDataFrame
+    def append(self, other, ignore_meta_conflict=False, inplace=False,
+               **kwargs):
+        """Append any castable object to this IamDataFrame.
+        Columns in `other.meta` that are not in `self.meta` are always merged,
+        duplicate region-variable-unit-year rows raise a ValueError.
 
         Parameters
         ----------
         other: pyam.IamDataFrame, ixmp.TimeSeries, ixmp.Scenario,
         pd.DataFrame or data file
-            an IamDataFrame, TimeSeries or Scenario (requires `ixmp`),
-            or pd.DataFrame or data file with IAMC-format data columns
+            An IamDataFrame, TimeSeries or Scenario (requires `ixmp`),
+            pandas.DataFrame or data file with IAMC-format data columns
+        ignore_meta_conflict : bool, default False
+            If False and `other` is an IamDataFrame, raise an error if
+            any meta columns present in `self` and `other` are not identical.
         inplace : bool, default False
-            if True, do operation inplace and return None
+            If True, do operation inplace and return None
         """
         ret = copy.deepcopy(self) if not inplace else self
 
         if not isinstance(other, IamDataFrame):
             other = IamDataFrame(other, **kwargs)
-
-        # check that any model/scenario is not yet included in IamDataFrame
-        ret.meta = ret.meta.append(other.meta, verify_integrity=True)
-
-        # add new data
-        ret.data = ret.data.append(other.data).reset_index(drop=True)
+            ignore_meta_conflict = True
+
+        diff = other.meta.index.difference(ret.meta.index)
+        intersect = other.meta.index.intersection(ret.meta.index)
+
+        # merge other.meta columns not in self.meta for existing scenarios
+        if not intersect.empty:
+            # if not ignored, check that overlapping meta dataframes are equal
+            if not ignore_meta_conflict:
+                cols = [i for i in other.meta.columns if i in ret.meta.columns]
+                if not ret.meta.loc[intersect, cols].equals(
+                        other.meta.loc[intersect, cols]):
+                    conflict_idx = (
+                        pd.concat([ret.meta.loc[intersect, cols],
+                                   other.meta.loc[intersect, cols]]
+                                  ).drop_duplicates()
+                        .index.drop_duplicates()
+                                   )
+                    msg = 'conflict in `meta` for scenarios {}'.format(
+                        [i for i in pd.DataFrame(index=conflict_idx).index])
+                    raise ValueError(msg)
+
+            cols = [i for i in other.meta.columns if i not in ret.meta.columns]
+            _meta = other.meta.loc[intersect, cols]
+            ret.meta = ret.meta.merge(_meta, how='outer',
+                                      left_index=True, right_index=True)
+
+        # join other.meta for new scenarios
+        if not diff.empty:
+            # sorting not supported by ` pd.append()`  prior to version 23
+            sort_kwarg = {} if int(pd.__version__.split('.')[1]) < 23 \
+                else dict(sort=False)
+            ret.meta = ret.meta.append(other.meta.loc[diff, :], **sort_kwarg)
+
+        # append other.data (verify integrity for no duplicates)
+        ret.data.set_index(LONG_IDX, inplace=True)
+        other.data.set_index(LONG_IDX, inplace=True)
+        ret.data = ret.data.append(other.data, verify_integrity=True)\
+            .reset_index(drop=False)
 
         if not inplace:
             return ret


Test patch:
diff --git a/tests/test_core.py b/tests/test_core.py
index 28c2674..e72e274 100644
--- a/tests/test_core.py
+++ b/tests/test_core.py
@@ -500,17 +500,65 @@ def test_load_RCP_database_downloaded_file(test_df):
     pd.testing.assert_frame_equal(obs_df.as_pandas(), test_df.as_pandas())
 
 
-def test_append(test_df):
-    df2 = test_df.append(other=os.path.join(
-        TEST_DATA_DIR, 'testing_data_2.csv'))
+def test_append_other_scenario(meta_df):
+    other = meta_df.filter(scenario='a_scenario2')\
+        .rename({'scenario': {'a_scenario2': 'a_scenario3'}})
+
+    meta_df.set_meta([0, 1], name='col1')
+    meta_df.set_meta(['a', 'b'], name='col2')
+
+    other.set_meta(2, name='col1')
+    other.set_meta('x', name='col3')
+
+    df = meta_df.append(other)
+
+    # check that the original meta dataframe is not updated
+    obs = meta_df.meta.index.get_level_values(1)
+    npt.assert_array_equal(obs, ['a_scenario', 'a_scenario2'])
+
+    # assert that merging of meta works as expected
+    exp = pd.DataFrame([
+        ['a_model', 'a_scenario', False, 0, 'a', np.nan],
+        ['a_model', 'a_scenario2', False, 1, 'b', np.nan],
+        ['a_model', 'a_scenario3', False, 2, np.nan, 'x'],
+    ], columns=['model', 'scenario', 'exclude', 'col1', 'col2', 'col3']
+    ).set_index(['model', 'scenario'])
+
+    # sort columns for assertion in older pandas versions
+    df.meta = df.meta.reindex(columns=exp.columns)
+    pd.testing.assert_frame_equal(df.meta, exp)
+
+    # assert that appending data works as expected
+    ts = df.timeseries()
+    npt.assert_array_equal(ts.iloc[2].values, ts.iloc[3].values)
+
+
+def test_append_same_scenario(meta_df):
+    other = meta_df.filter(scenario='a_scenario2')\
+        .rename({'variable': {'Primary Energy': 'Primary Energy clone'}})
+
+    meta_df.set_meta([0, 1], name='col1')
+
+    other.set_meta(2, name='col1')
+    other.set_meta('b', name='col2')
+
+    # check that non-matching meta raise an error
+    pytest.raises(ValueError, meta_df.append, other=other)
+
+    # check that ignoring meta conflict works as expetced
+    df = meta_df.append(other, ignore_meta_conflict=True)
 
     # check that the new meta.index is updated, but not the original one
-    obs = test_df.meta.index.get_level_values(1)
-    npt.assert_array_equal(obs, ['a_scenario'])
+    npt.assert_array_equal(meta_df.meta.columns, ['exclude', 'col1'])
+
+    # assert that merging of meta works as expected
+    exp = meta_df.meta.copy()
+    exp['col2'] = [np.nan, 'b']
+    pd.testing.assert_frame_equal(df.meta, exp)
 
-    exp = ['a_scenario', 'append_scenario']
-    obs2 = df2.meta.index.get_level_values(1)
-    npt.assert_array_equal(obs2, exp)
+    # assert that appending data works as expected
+    ts = df.timeseries()
+    npt.assert_array_equal(ts.iloc[2], ts.iloc[3])
 
 
 def test_append_duplicates(test_df):