Repository: IAMconsortium/pyam

Issue:
concat uses pandas append instead of concat
The implementation of pyams concat method loops over pandas append instead of using pandas concat method. Especially for large dataframes with subannual data concat is much faster than append.

I already implemented pandas concat in pyams concat and will push it in a minute. I hope the new feature will find its way in the next release.

Discussion:
danielhuppmann: @pjuergens, it would be great if you could try out this implementation for your use cases and report any unexpected issues. Also, it would be interesting to hear about the performance improvements in a real-world application.
codecov[bot]: # [Codecov](https://codecov.io/gh/IAMconsortium/pyam/pull/510?src=pr&el=h1) Report
> Merging [#510](https://codecov.io/gh/IAMconsortium/pyam/pull/510?src=pr&el=desc) (bae4c68) into [main](https://codecov.io/gh/IAMconsortium/pyam/commit/a9e93e0ab8754b29c0e6453aa87e317435165f98?el=desc) (a9e93e0) will **decrease** coverage by `0.0%`.
> The diff coverage is `92.3%`.

[![Impacted file tree graph](https://codecov.io/gh/IAMconsortium/pyam/pull/510/graphs/tree.svg?width=650&height=150&src=pr&token=ThxWlNkonJ)](https://codecov.io/gh/IAMconsortium/pyam/pull/510?src=pr&el=tree)

```diff
@@           Coverage Diff           @@
##            main    #510     +/-   ##
=======================================
- Coverage   93.0%   93.0%   -0.1%     
=======================================
  Files         43      43             
  Lines       4876    4886     +10     
=======================================
+ Hits        4537    4545      +8     
- Misses       339     341      +2     
```


| [Impacted Files](https://codecov.io/gh/IAMconsortium/pyam/pull/510?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pyam/core.py](https://codecov.io/gh/IAMconsortium/pyam/pull/510/diff?src=pr&el=tree#diff-cHlhbS9jb3JlLnB5) | `92.1% <88.8%> (-0.2%)` | :arrow_down: |
| [tests/test\_core.py](https://codecov.io/gh/IAMconsortium/pyam/pull/510/diff?src=pr&el=tree#diff-dGVzdHMvdGVzdF9jb3JlLnB5) | `100.0% <100.0%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/IAMconsortium/pyam/pull/510?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/IAMconsortium/pyam/pull/510?src=pr&el=footer). Last update [a9e93e0...bae4c68](https://codecov.io/gh/IAMconsortium/pyam/pull/510?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

pjuergens: @danielhuppmann thanks for working on this issue. I'll try to test the implementation today.

Concerning performance-improvement in real-world-application: We have a code in which we aggregate a lot of hourly-resolved timeseries. The bottleneck there was the pyam.append-function. Switching from append to concat on major parts of the code with my original implementation I could reduce the calculation time from 24 to 15 minutes. I'll have a deeper look at it again today when I test the new implementation.
pjuergens: my script ran without any issues, thanks again!

danielhuppmann: At some point, it will be so fast that you don't even have time for coffee while your scripts are running 😜...

Patch:
diff --git a/AUTHORS.rst b/AUTHORS.rst
index 106aae8..f49b3df 100644
--- a/AUTHORS.rst
+++ b/AUTHORS.rst
@@ -17,3 +17,4 @@ The following persons contributed to the development of the |pyam| framework:
 - Maik Budzinski `@mabudz <https://github.com/mabudz>`_
 - Jarmo Kikstra `@jkikstra <https://github.com/jkikstra>`_
 - Michael Pimmer `@fonfon <https://github.com/fonfon>`_
+- Patrick Jürgens `@pjuergens <https://github.com/pjuergens>`_
diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md
index d016113..7bf5b1e 100644
--- a/RELEASE_NOTES.md
+++ b/RELEASE_NOTES.md
@@ -8,6 +8,7 @@ starting with `data` will be parsed for timeseries data.
 
 ## Individual updates
 
+- [#510](https://github.com/IAMconsortium/pyam/pull/510) Improve performance of `pyam.concat()`
 - [#508](https://github.com/IAMconsortium/pyam/pull/508) Bugfix for non-empty but invisible header and no rows in 'meta' sheet
 - [#502](https://github.com/IAMconsortium/pyam/pull/502) Switch to Black code style
 - [#499](https://github.com/IAMconsortium/pyam/pull/499) Implement `order` feature in line plot 
diff --git a/pyam/core.py b/pyam/core.py
index cff1cc3..23531f5 100755
--- a/pyam/core.py
+++ b/pyam/core.py
@@ -500,7 +500,7 @@ class IamDataFrame(object):
         Raises
         ------
         ValueError
-            If time domain or other timeseries data index dimension don't match
+            If time domain or other timeseries data index dimension don't match.
         """
         if not isinstance(other, IamDataFrame):
             other = IamDataFrame(other, **kwargs)
@@ -2306,28 +2306,61 @@ def compare(
     return ret[[right_label, left_label]]
 
 
-def concat(dfs):
+def concat(dfs, ignore_meta_conflict=False, **kwargs):
     """Concatenate a series of IamDataFrame-like objects
 
     Parameters
     ----------
     dfs : list of IamDataFrames
-        a list of :class:`IamDataFrame` instances
+        A list of :class:`IamDataFrame` instances
+    ignore_meta_conflict : bool, default False
+        If False, raise an error if any meta columns present in `dfs` are not identical.
+        If True, values in earlier elements of `dfs` take precendence.
+    kwargs
+        Passed to :class:`IamDataFrame(other, **kwargs) <IamDataFrame>`
+        for any item of `dfs` which isn't already an IamDataFrame.
+
+    Returns
+    -------
+    IamDataFrame
+
+    Raises
+    ------
+    TypeError
+        If `dfs` is not a list.
+    ValueError
+        If time domain or other timeseries data index dimension don't match.
     """
-    if isstr(dfs) or not hasattr(dfs, "__iter__"):
-        msg = "Argument must be a non-string iterable (e.g., list or tuple)"
-        raise TypeError(msg)
+    if not islistable(dfs) or isinstance(dfs, pd.DataFrame):
+        raise TypeError(
+            f"First argument must be an iterable, "
+            f"you passed an object of type '{dfs.__class__.__name__}'!"
+        )
 
-    _df = None
-    for df in dfs:
-        df = df if isinstance(df, IamDataFrame) else IamDataFrame(df)
-        if _df is None:
-            _df = df.copy()
-        else:
-            _df.append(df, inplace=True, verify_integrity=False)
+    # cast first element in list to IamDataFrame (if necessary)
+    df = dfs[0] if isinstance(dfs[0], IamDataFrame) else IamDataFrame(dfs[0], **kwargs)
+    ret_data, ret_meta = [df._data], df.meta
+    index, time_col = df._data.index.names, df.time_col
+
+    for df in dfs[1:]:
+        # skip merging meta if element is a pd.DataFrame
+        _meta_merge = False if isinstance(df, pd.DataFrame) else True
+        df = IamDataFrame(df, **kwargs) if not isinstance(df, IamDataFrame) else df
+
+        if df.time_col != time_col:
+            raise ValueError("Items have incompatible time format ('year' vs. 'time')!")
+
+        if df._data.index.names != index:
+            raise ValueError(
+                "Items have incompatible timeseries data index dimensions!"
+            )
+
+        ret_data.append(df._data)
+        if _meta_merge:
+            ret_meta = merge_meta(ret_meta, df.meta, ignore_meta_conflict)
 
-    verify_index_integrity(_df._data)
-    return _df
+    # return as new IamDataFrame, this will verify integrity as part of `__init__()`
+    return IamDataFrame(pd.concat(ret_data, verify_integrity=False), meta=ret_meta)
 
 
 def read_datapackage(path, data="data", meta="meta"):


Test patch:
diff --git a/tests/conftest.py b/tests/conftest.py
index 72b138b..9acf500 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -148,8 +148,9 @@ def test_df_year():
 # minimal IamDataFrame for specifically testing 'time'-column features
 @pytest.fixture(scope="function")
 def test_df_time():
-    tdf = TEST_DF.rename({2005: TEST_DTS[0], 2010: TEST_DTS[1]}, axis="columns")
-    df = IamDataFrame(data=tdf)
+    df = IamDataFrame(
+        data=TEST_DF.rename({2005: TEST_DTS[0], 2010: TEST_DTS[1]}, axis="columns")
+    )
     for i in META_COLS:
         df.set_meta(META_DF[i])
     yield df
diff --git a/tests/test_core.py b/tests/test_core.py
index f6cf140..9ee6106 100644
--- a/tests/test_core.py
+++ b/tests/test_core.py
@@ -1027,12 +1027,37 @@ def test_pd_join_by_meta_nonmatching_index(test_df):
     pd.testing.assert_frame_equal(obs.sort_index(level=1), exp)
 
 
-def test_concat_fails_iter():
-    pytest.raises(TypeError, concat, 1)
+def test_concat_fails_iterable(test_pd_df):
+    """Check that calling concat with a non-iterable raises"""
+    msg = "First argument must be an iterable, you passed an object of type '{}'!"
 
+    for dfs, type_ in [(1, "int"), ("foo", "str"), (test_pd_df, "DataFrame")]:
+        with pytest.raises(TypeError, match=msg.format(type_)):
+            concat(dfs)
 
-def test_concat_fails_notdf():
-    pytest.raises(TypeError, concat, "foo")
+
+def test_concat_single_item(test_df):
+    """Check that calling concat on a single-item list returns identical object"""
+    obs = concat([test_df])
+    assert_iamframe_equal(obs, test_df)
+
+
+def test_concat_incompatible_time(test_df_year, test_df_time):
+    """Check that calling concat with incompatible time formats raises"""
+    match = re.escape("Items have incompatible time format ('year' vs. 'time')!")
+    with pytest.raises(ValueError, match=match):
+        concat([test_df_year, test_df_time])
+
+
+def test_concat_incompatible_cols(test_pd_df):
+    """Check that calling concat on a single-item list returns identical object"""
+    df1 = IamDataFrame(test_pd_df)
+    test_pd_df["extra_col"] = "foo"
+    df2 = IamDataFrame(test_pd_df)
+
+    match = "Items have incompatible timeseries data index dimensions!"
+    with pytest.raises(ValueError, match=match):
+        concat([df1, df2])
 
 
 def test_concat(test_df):