Skip to content
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.

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

Multiple Functions require lists of Feols/Feiv/Fepois object; reject FixestMulti #693 #715

Merged
merged 10 commits into from
Nov 17, 2024
70,931 changes: 3,694 additions & 67,237 deletions coverage.xml

Large diffs are not rendered by default.

1,332 changes: 666 additions & 666 deletions pixi.lock

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion pyfixest/estimation/FixestMulti_.py
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@ def _estimate_all_models(

self.all_fitted_models[FIT._model_name] = FIT

def to_list(self):
def to_list(self) -> list[Union[Feols, Fepois, Feiv]]:
"""
Return a list of all fitted models.

Expand Down
2 changes: 2 additions & 0 deletions pyfixest/estimation/feols_.py
Original file line number Diff line number Diff line change
Expand Up @@ -1696,10 +1696,12 @@ def get_performance(self) -> None:
ssy = np.sum((_Y - np.mean(_Y)) ** 2)

if _has_weights:
ssy = np.sum(_Y**2) - np.sum(_Y**2) / np.sum(_weights)
self._rmse = np.nan
self._r2 = np.nan
self._adj_r2 = np.nan
else:
ssy = np.sum((_Y - np.mean(_Y)) ** 2)
self._rmse = np.sqrt(ssu / _N)
self._r2 = 1 - (ssu / ssy)
self._adj_r2 = 1 - (ssu / ssy) * _adj_factor
Expand Down
13 changes: 8 additions & 5 deletions pyfixest/estimation/multcomp.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,16 @@
import numpy as np
import pandas as pd

from pyfixest.estimation.feiv_ import Feiv
from pyfixest.estimation.feols_ import Feols
from pyfixest.estimation.fepois_ import Fepois
from pyfixest.estimation.FixestMulti_ import FixestMulti
from pyfixest.report.summarize import _post_processing_input_checks

ModelInputType = Union[FixestMulti, list[Union[Feols, Fepois, Feiv]]]

def bonferroni(models: list[Union[Feols, Fepois]], param: str) -> pd.DataFrame:

def bonferroni(models: ModelInputType, param: str) -> pd.DataFrame:
"""
Compute Bonferroni adjusted p-values for multiple hypothesis testing.

Expand All @@ -18,9 +22,8 @@ def bonferroni(models: list[Union[Feols, Fepois]], param: str) -> pd.DataFrame:

Parameters
----------
models : list[Feols, Fepois], Feols or Fepois
A list of models for which the p-values should be adjusted, or a Feols or
Fepois object.
models : A supported model object (Feols, Fepois, Feiv, FixestMulti) or a list of
Feols, Fepois & Feiv models.
param : str
The parameter for which the p-values should be adjusted.

Expand Down Expand Up @@ -65,7 +68,7 @@ def bonferroni(models: list[Union[Feols, Fepois]], param: str) -> pd.DataFrame:


def rwolf(
models: list[Union[Feols, Fepois]],
models: Union[FixestMulti, list[Union[Feols, Fepois]]],
param: str,
reps: int,
seed: int,
Expand Down
65 changes: 26 additions & 39 deletions pyfixest/report/summarize.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import re
from collections.abc import ValuesView
from typing import Optional, Union

import numpy as np
Expand All @@ -13,9 +14,13 @@
from pyfixest.report.utils import _relabel_expvar
from pyfixest.utils.dev_utils import _select_order_coefs

ModelInputType = Union[
FixestMulti, Feols, Fepois, Feiv, list[Union[Feols, Fepois, Feiv]]
]


def etable(
models: Union[list[Union[Feols, Fepois, Feiv]], FixestMulti],
models: ModelInputType,
type: str = "gt",
signif_code: Optional[list] = None,
coef_fmt: str = "b \n (se)",
Expand All @@ -40,8 +45,8 @@

Parameters
----------
models : list
A list of models of type Feols, Feiv, Fepois.
models : A supported model object (Feols, Fepois, Feiv, FixestMulti) or a list of
Feols, Fepois & Feiv models.
type : str, optional
Type of output. Either "df" for pandas DataFrame, "md" for markdown,
"gt" for great_tables, or "tex" for LaTeX table. Default is "gt".
Expand Down Expand Up @@ -132,11 +137,6 @@
signif_code[0] < signif_code[1] < signif_code[2]
), "signif_code must be in increasing order"

# Check if models is of type FixestMulti
# If so, convert it to a list of models
if isinstance(models, FixestMulti):
models = models.to_list()

models = _post_processing_input_checks(models)

if custom_stats is None:
Expand Down Expand Up @@ -464,9 +464,7 @@
return None


def summary(
models: Union[list[Union[Feols, Fepois, Feiv]], FixestMulti], digits: int = 3
) -> None:
def summary(models: ModelInputType, digits: int = 3) -> None:
"""
Print a summary of estimation results for each estimated model.

Expand All @@ -476,8 +474,8 @@

Parameters
----------
models : list[Union[Feols, Fepois, Feiv]] or FixestMulti.
The models to be summarized.
models : A supported model object (Feols, Fepois, Feiv, FixestMulti) or a list of
Feols, Fepois & Feiv models.
digits : int, optional
The number of decimal places to round the summary statistics to. Default is 3.

Expand Down Expand Up @@ -550,8 +548,8 @@


def _post_processing_input_checks(
models: Union[list[Union[Feols, Fepois, Feiv]], FixestMulti],
) -> list[Union[Feols, Fepois]]:
models: ModelInputType,
) -> list[Union[Feols, Fepois, Feiv]]:
"""
Perform input checks for post-processing models.

Expand All @@ -572,34 +570,23 @@
TypeError: If the models argument is not of the expected type.

"""
# check if models instance of Feols or Fepois
if isinstance(models, (Feols, Fepois)):
models = [models]

else:
if isinstance(models, (list, type({}.values()))):
for model in models:
if not isinstance(model, (Feols, Fepois)):
raise TypeError(
f"""
Each element of the passed list needs to be of type Feols
or Fepois, but {type(model)} was passed. If you want to
summarize a FixestMulti object, please use FixestMulti.to_list()
to convert it to a list of Feols or Fepois instances.
"""
)

models_list: list[Union[Feols, Fepois, Feiv]] = []

if isinstance(models, (Feols, Fepois, Feiv)):
models_list = [models]
elif isinstance(models, FixestMulti):
models_list = models.to_list()
elif isinstance(models, (list, ValuesView)):
if all(isinstance(m, (Feols, Fepois, Feiv)) for m in models):
models_list = models
else:
raise TypeError(
"""
The models argument must be either a list of Feols or Fepois instances, or
simply a single Feols or Fepois instance. The models argument does not accept instances
of type FixestMulti - please use models.to_list() to convert the FixestMulti
instance to a list of Feols or Fepois instances.
"""
"All elements in the models list must be instances of Feols, Feiv, or Fepois."
)
else:
raise TypeError("Invalid type for models argument.")

Check warning on line 587 in pyfixest/report/summarize.py

View check run for this annotation

Codecov / codecov/patch

pyfixest/report/summarize.py#L587

Added line #L587 was not covered by tests

return models
return models_list


def _tabulate_etable_md(df, n_coef, n_fixef, n_models, n_model_stats):
Expand Down
23 changes: 14 additions & 9 deletions pyfixest/report/visualize.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,16 @@
from pyfixest.estimation.feiv_ import Feiv
from pyfixest.estimation.feols_ import Feols
from pyfixest.estimation.fepois_ import Fepois
from pyfixest.estimation.FixestMulti_ import FixestMulti
from pyfixest.report.summarize import _post_processing_input_checks
from pyfixest.report.utils import _relabel_expvar
from pyfixest.utils.dev_utils import _select_order_coefs

ModelInputType = Union[
FixestMulti, Feols, Fepois, Feiv, list[Union[Feols, Fepois, Feiv]]
]


LetsPlot.setup_html()


Expand Down Expand Up @@ -60,7 +66,7 @@ def set_figsize(


def iplot(
models,
models: ModelInputType,
alpha: float = 0.05,
figsize: Optional[tuple[int, int]] = None,
yintercept: Union[int, str, None] = None,
Expand All @@ -83,9 +89,8 @@ def iplot(

Parameters
----------
models : list or object
A list of fitted models of type `Feols` or
`Fepois`, or just a single model.
models : A supported model object (Feols, Fepois, Feiv, FixestMulti) or a list of
Feols, Fepois & Feiv models.
figsize : tuple or None, optional
The size of the figure. If None, the default size is used.
alpha : float
Expand Down Expand Up @@ -157,8 +162,8 @@ def iplot(
"The 'joint' parameter is only available for a single model, i.e. objects of type FixestMulti are not supported."
)

df_all = []
all_icovars = []
df_all: list[pd.DataFrame] = []
all_icovars: list[str] = []

if keep is None:
keep = []
Expand Down Expand Up @@ -205,7 +210,7 @@ def iplot(


def coefplot(
models: list,
models: ModelInputType,
alpha: float = 0.05,
figsize: Optional[tuple[int, int]] = None,
yintercept: float = 0,
Expand All @@ -227,8 +232,8 @@ def coefplot(

Parameters
----------
models : list or object
A list of fitted models of type `Feols` or `Fepois`, or just a single model.
models : A supported model object (Feols, Fepois, Feiv, FixestMulti) or a list of
Feols, Fepois & Feiv models.
figsize : tuple or None, optional
The size of the figure. If None, the default size is used.
alpha : float
Expand Down
6 changes: 6 additions & 0 deletions tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,17 @@ def test_api():
fit2 = pf.estimation.fepois(
"Y ~ X1 + X2 + f2 | f1", data=df2, vcov={"CRV1": "f1+f2"}
)
fit_multi = pf.feols("Y + Y2 ~ X1", data=df2)

pf.summary(fit1)
pf.report.summary(fit2)
pf.etable([fit1, fit2])
pf.coefplot([fit1, fit2])

pf.summary(fit_multi)
pf.etable(fit_multi)
pf.coefplot(fit_multi)


def test_feols_args():
"""
Expand Down
18 changes: 18 additions & 0 deletions tests/test_multcomp.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from rpy2.robjects import pandas2ri
from rpy2.robjects.packages import importr

import pyfixest as pf
from pyfixest.estimation.estimation import feols
from pyfixest.estimation.multcomp import _get_rwolf_pval, bonferroni, rwolf
from pyfixest.utils.set_rpy2_path import update_r_paths
Expand Down Expand Up @@ -195,3 +196,20 @@ def test_sampling_scheme(seed, reps):
assert (
np.abs(percent_diff) < 1.0
), f"Percentage difference is too large: {percent_diff}%"


@pytest.mark.extended
def test_multi_vs_list(seeds, reps):
"Test that lists of models and FixestMulti input produce identical results."
seed = 1232
reps = 100
data = pf.get_data(N=100)

fit_all = pf.feols("Y + Y2 ~ X1 + X2", data=data)
fit1 = pf.feols("Y ~ X1 + X2", data=data)
fit2 = pf.feols("Y2 ~ X1 + X2", data=data)

assert bonferroni(fit_all, "X1").equals(bonferroni([fit1, fit2], "X1"))
assert rwolf(fit_all, "X1", seed=seed, reps=reps).equals(
rwolf([fit1, fit2], "X1", seed=seed, reps=reps)
)
Loading