Repository: IAMconsortium/pyam
Pull request #734 (2023-02-28 15:36:01)

Validation for illegal column names in `data`
# Please confirm that this PR has done the following:

- [x] Tests Added
- ~Documentation Added~
- ~Name of contributors Added to AUTHORS.rst~
- [x] Description in RELEASE_NOTES.md Added

# Description of PR

During review of #729, I noticed that there may be issues when initializing an IamDataFrame with an unnamed column (None or empty string) - both could give unexpected behavior. This PR adds these two corner cases to the validation routine.


codecov[bot]: # [Codecov](https://codecov.io/gh/IAMconsortium/pyam/pull/734?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=IAMconsortium) Report
> Merging [#734](https://codecov.io/gh/IAMconsortium/pyam/pull/734?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=IAMconsortium) (7318209) into [main](https://codecov.io/gh/IAMconsortium/pyam/commit/e07d3b91e2fc06819db17080013b271591021714?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=IAMconsortium) (e07d3b9) will **increase** coverage by `0.0%`.
> The diff coverage is `98.2%`.

```diff
@@          Coverage Diff          @@
##            main    #734   +/-   ##
=====================================
  Coverage   95.0%   95.1%           
=====================================
  Files         59      59           
  Lines       6020    6044   +24     
=====================================
+ Hits        5725    5748   +23     
- Misses       295     296    +1     
```


| [Impacted Files](https://codecov.io/gh/IAMconsortium/pyam/pull/734?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=IAMconsortium) | Coverage Δ | |
|---|---|---|
| [pyam/utils.py](https://codecov.io/gh/IAMconsortium/pyam/pull/734?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=IAMconsortium#diff-cHlhbS91dGlscy5weQ==) | `92.7% <97.8%> (+0.1%)` | :arrow_up: |
| [tests/test\_core.py](https://codecov.io/gh/IAMconsortium/pyam/pull/734?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=IAMconsortium#diff-dGVzdHMvdGVzdF9jb3JlLnB5) | `100.0% <100.0%> (ø)` | |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=IAMconsortium). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=IAMconsortium)


Patch:
diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md
index d997366..b98ba7b 100644
--- a/RELEASE_NOTES.md
+++ b/RELEASE_NOTES.md
@@ -1,5 +1,6 @@
 # Next Release
 
+- [#734](https://github.com/IAMconsortium/pyam/pull/734) Validation for illegal column names in `data`
 - [#731](https://github.com/IAMconsortium/pyam/pull/731) Add fast-path to initialization for sufficient multiindex
 - [#732](https://github.com/IAMconsortium/pyam/pull/732) Fix a few typos in tutorials
 - [#730](https://github.com/IAMconsortium/pyam/pull/730) Refactor initialization code
diff --git a/pyam/utils.py b/pyam/utils.py
index 721b4cc..7d46e2e 100644
--- a/pyam/utils.py
+++ b/pyam/utils.py
@@ -26,7 +26,7 @@ LONG_IDX = IAMC_IDX + ["year"]
 REQUIRED_COLS = ["region", "variable", "unit"]
 
 # illegal terms for data/meta column names to prevent attribute conflicts
-ILLEGAL_COLS = ["data", "meta", "level"]
+ILLEGAL_COLS = ["data", "meta", "level", ""]
 
 # dictionary to translate column count to Excel column names
 NUMERIC_TO_STR = dict(
@@ -262,12 +262,19 @@ def _intuit_column_groups(df, index, include_index=False):
         existing_cols = existing_cols.union(df.columns)
 
     # check that there is no column in the timeseries data with reserved names
+    if None in existing_cols:
+        raise ValueError("Unnamed column in `data`: None")
+
+    # check that there is no column in the timeseries data with reserved/illegal names
     conflict_cols = [i for i in existing_cols if i in ILLEGAL_COLS]
     if conflict_cols:
-        msg = f"Column name {conflict_cols} is illegal for timeseries data.\n"
-        _args = ", ".join([f"{i}_1='{i}'" for i in conflict_cols])
-        msg += f"Use `IamDataFrame(..., {_args})` to rename at initialization."
-        raise ValueError(msg)
+        sep = "', '"
+        _cols = f"'{sep.join(conflict_cols)}'"
+        _args = ", ".join([f"<alternative_column_name>='{i}'" for i in conflict_cols])
+        raise ValueError(
+            f"Illegal column{s(len(conflict_cols))} for timeseries data: {_cols}\n"
+            f"Use `IamDataFrame(..., {_args})` to rename at initialization."
+        )
 
     # check that index and required columns exist
     missing_index = [c for c in index if c not in existing_cols]


Test patch:
diff --git a/tests/test_core.py b/tests/test_core.py
index 939bb08..c70b7d5 100644
--- a/tests/test_core.py
+++ b/tests/test_core.py
@@ -189,18 +189,28 @@ def test_init_empty_message(caplog):
     assert caplog.records[message_idx].levelno == logging.WARNING
 
 
-def test_init_with_column_conflict(test_pd_df):
-    # add a column to the timeseries data with a conflict to the meta attribute
-    test_pd_df["meta"] = "foo"
+def test_init_with_unnamed_column(test_pd_df):
+    # add a column to the timeseries data with an unnamed column
+    test_pd_df[None] = "foo"
 
-    # check that initialising an instance with an extra-column `meta` raises
-    msg = re.compile(r"Column name \['meta'\] is illegal for timeseries data.")
+    # check that initialising an instance with an unnamed column raises
+    with pytest.raises(ValueError, match="Unnamed column in `data`: None"):
+        IamDataFrame(test_pd_df)
+
+
+@pytest.mark.parametrize("illegal", ["meta", ""])
+def test_init_with_illegal_column(test_pd_df, illegal):
+    # add a column to the timeseries data with an illegal column name
+    test_pd_df[illegal] = "foo"
+
+    # check that initialising an instance with an illegal column name raises
+    msg = f"Illegal column for timeseries data: '{illegal}'"
     with pytest.raises(ValueError, match=msg):
         IamDataFrame(test_pd_df)
 
     # check that recommended fix works
-    df = IamDataFrame(test_pd_df, meta_1="meta")
-    assert df.meta_1 == ["foo"]
+    df = IamDataFrame(test_pd_df, valid=illegal)
+    assert df.valid == ["foo"]
 
 
 def test_set_meta_with_column_conflict(test_df_year):