Skip to content

Narrow aggregation exception guards to TypeError #311

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
vahid-ahmadi wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Narrow aggregation exception guards to TypeError #311

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions changelog.d/narrow-aggregation-exceptions.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
- bump: patch
changes:
fixed:
- DataFrame-level aggregations no longer swallow every exception per
column, so genuine argument errors raise instead of returning a
silently truncated result.
82 changes: 60 additions & 22 deletions microdf/microdataframe.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,9 +150,13 @@ def fn(*args, **kwargs) -> pd.Series:
if pd.api.types.is_numeric_dtype(self[col]):
try:
results[col] = getattr(self[col], name)(*args, **kwargs)
except Exception:
# Skip columns that can't be aggregated
pass
except TypeError as exc:
# Skip columns whose dtype can't take this aggregation.
# Deliberately narrow: catching every Exception here also
# swallowed real errors (e.g. the ValueError from
# gini(negatives=...)) and returned a silently truncated
# result instead of raising.
logger.debug("skipping column %s in %s: %s", col, name, exc)
return pd.Series(results)

return fn
Expand All @@ -173,9 +177,13 @@ def fn(*args, **kwargs) -> pd.DataFrame:
result = getattr(self[col], name)(*args, **kwargs)
results.append(result)
columns.append(col)
except Exception:
# Skip columns that can't be aggregated
pass
except TypeError as exc:
# Skip columns whose dtype can't take this aggregation.
# Deliberately narrow: catching every Exception here also
# swallowed real errors (e.g. the ValueError from
# gini(negatives=...)) and returned a silently truncated
# result instead of raising.
logger.debug("skipping column %s in %s: %s", col, name, exc)

if results:
df = pd.DataFrame(results)
Expand Down Expand Up @@ -208,9 +216,13 @@ def fn(*args, **kwargs) -> Union[pd.Series, pd.DataFrame]:
result = getattr(self[col], name)(*args, **kwargs)
results.append(result)
columns.append(col)
except Exception:
# Skip columns that can't be aggregated
pass
except TypeError as exc:
# Skip columns whose dtype can't take this aggregation.
# Deliberately narrow: catching every Exception here also
# swallowed real errors (e.g. the ValueError from
# gini(negatives=...)) and returned a silently truncated
# result instead of raising.
logger.debug("skipping column %s in %s: %s", col, name, exc)

if results:
df = pd.DataFrame(results)
Expand All @@ -225,9 +237,13 @@ def fn(*args, **kwargs) -> Union[pd.Series, pd.DataFrame]:
if pd.api.types.is_numeric_dtype(self[col]):
try:
results[col] = getattr(self[col], name)(*args, **kwargs)
except Exception:
# Skip columns that can't be aggregated
pass
except TypeError as exc:
# Skip columns whose dtype can't take this aggregation.
# Deliberately narrow: catching every Exception here also
# swallowed real errors (e.g. the ValueError from
# gini(negatives=...)) and returned a silently truncated
# result instead of raising.
logger.debug("skipping column %s in %s: %s", col, name, exc)
return pd.Series(results)

return fn
Expand Down Expand Up @@ -836,9 +852,13 @@ def fn(*args, **kwargs):
results[col] = getattr(getattr(self, col), name)(
*args, **kwargs
)
except Exception:
# Skip columns that can't be aggregated
pass
except TypeError as exc:
# Skip columns whose dtype can't take this aggregation.
# Deliberately narrow: catching every Exception here also
# swallowed real errors (e.g. the ValueError from
# gini(negatives=...)) and returned a silently truncated
# result instead of raising.
logger.debug("skipping column %s in %s: %s", col, name, exc)
# Return plain DataFrame - aggregated results don't have
# per-row weights (weights were already applied)
return pd.DataFrame(results) if results else pd.DataFrame()
Expand All @@ -856,9 +876,13 @@ def fn(*args, **kwargs) -> Union[pd.Series, pd.DataFrame]:
results[col] = getattr(getattr(self, col), name)(
*args, **kwargs
)
except Exception:
# Skip columns that can't be aggregated
pass
except TypeError as exc:
# Skip columns whose dtype can't take this aggregation.
# Deliberately narrow: catching every Exception here also
# swallowed real errors (e.g. the ValueError from
# gini(negatives=...)) and returned a silently truncated
# result instead of raising.
logger.debug("skipping column %s in %s: %s", col, name, exc)
# Return plain DataFrame - aggregated results don't have
# per-row weights (weights were already applied)
return pd.DataFrame(results) if results else pd.DataFrame()
Expand Down Expand Up @@ -918,8 +942,15 @@ def fn(*args, **kwargs):
results[col] = getattr(getattr(res, col), name)(
*args, **kwargs
)
except Exception:
pass
except TypeError as exc:
# Skip columns whose dtype can't take this aggregation.
# Deliberately narrow: catching every Exception here also
# swallowed real errors (e.g. the ValueError from
# gini(negatives=...)) and returned a silently truncated
# result instead of raising.
logger.debug(
"skipping column %s in %s: %s", col, name, exc
)
# Return plain DataFrame - aggregated results don't
# have per-row weights (weights were already applied)
return pd.DataFrame(results) if results else pd.DataFrame()
Expand All @@ -937,8 +968,15 @@ def fn(*args, **kwargs):
results[col] = getattr(getattr(res, col), name)(
*args, **kwargs
)
except Exception:
pass
except TypeError as exc:
# Skip columns whose dtype can't take this aggregation.
# Deliberately narrow: catching every Exception here also
# swallowed real errors (e.g. the ValueError from
# gini(negatives=...)) and returned a silently truncated
# result instead of raising.
logger.debug(
"skipping column %s in %s: %s", col, name, exc
)
# Return plain DataFrame - aggregated results don't
# have per-row weights (weights were already applied)
return pd.DataFrame(results) if results else pd.DataFrame()
Expand Down
24 changes: 24 additions & 0 deletions microdf/tests/test_microseries_dataframe.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -815,3 +815,27 @@ def test_rank_ties_share_bucket() -> None:
# existing ``test_rank`` expectations hold.
s = mdf.MicroSeries([1, 2, 3], weights=[4, 5, 6])
np.testing.assert_array_equal(s.rank().values, [4, 9, 15])


def test_aggregation_surfaces_real_errors():
"""A genuine argument error must raise, not be swallowed per column."""
df = mdf.MicroDataFrame(pd.DataFrame({"x": [1, 2, 3]}), weights=[1, 1, 1])
with pytest.raises(ValueError, match="Unknown negatives option"):
df.gini(negatives="bogus")


def test_aggregation_still_skips_non_numeric_columns():
"""Narrowing the guard must not change which columns aggregate."""
df = mdf.MicroDataFrame(
pd.DataFrame(
{
"x": [1, 2, 3],
"s": ["a", "b", "c"],
"dt": pd.to_datetime(["2020-01-01"] * 3),
}
),
weights=[1, 2, 3],
)
assert list(df.sum().index) == ["x"]
assert df.sum()["x"] == 14
assert list(df.mean().index) == ["x"]
Loading