diff --git a/modelskill/comparison/_collection.py b/modelskill/comparison/_collection.py index c6d9057ea..391175b3c 100644 --- a/modelskill/comparison/_collection.py +++ b/modelskill/comparison/_collection.py @@ -30,7 +30,7 @@ from ..skill import SkillTable from ..skill_grid import SkillGrid -from ..utils import _get_idx, _get_name +from ..utils import _get_name from ._comparison import Comparer, Scoreable from ..metrics import _parse_metric from ._utils import ( @@ -40,31 +40,6 @@ IdxOrNameTypes, TimeTypes, ) -from ._comparison import _get_deprecated_args # TODO remove in v 1.1 - - -def _get_deprecated_obs_var_args(kwargs): # type: ignore - observation, variable = None, None - - # Don't bother refactoring this, it will be removed in v1.1 - if "observation" in kwargs: - observation = kwargs.pop("observation") - if observation is not None: - warnings.warn( - f"The 'observation' argument is deprecated, use 'sel(observation='{observation}') instead", - FutureWarning, - ) - - if "variable" in kwargs: - variable = kwargs.pop("variable") - - if variable is not None: - warnings.warn( - f"The 'variable' argument is deprecated, use 'sel(quantity='{variable}') instead", - FutureWarning, - ) - - return observation, variable class ComparerCollection(Mapping, Scoreable): @@ -446,7 +421,6 @@ def skill( by: str | Iterable[str] | None = None, metrics: Iterable[str] | Iterable[Callable] | str | Callable | None = None, observed: bool = False, - **kwargs: Any, ) -> SkillTable: """Aggregated skill assessment of model(s) @@ -505,36 +479,18 @@ def skill( 2017-10-29 163 -0.21 0.52 0.47 0.42 0.79 0.11 0.99 """ - # TODO remove in v1.1 ---------- - model, start, end, area = _get_deprecated_args(kwargs) # type: ignore - observation, variable = _get_deprecated_obs_var_args(kwargs) # type: ignore - assert kwargs == {}, f"Unknown keyword arguments: {kwargs}" - - cc = self.sel( - model=model, - observation=observation, - quantity=variable, - start=start, - end=end, - area=area, - ) - if cc.n_points == 0: - raise ValueError("Dataset is empty, no data to compare.") - - ## ---- end of deprecated code ---- - pmetrics = _parse_metric(metrics) - agg_cols = _parse_groupby(by, n_mod=cc.n_models, n_qnt=cc.n_quantities) + agg_cols = _parse_groupby(by, n_mod=self.n_models, n_qnt=self.n_quantities) agg_cols, attrs_keys = self._attrs_keys_in_by(agg_cols) - df = cc._to_long_dataframe(attrs_keys=attrs_keys, observed=observed) + df = self._to_long_dataframe(attrs_keys=attrs_keys, observed=observed) res = _groupby_df(df, by=agg_cols, metrics=pmetrics) mtr_cols = [m.__name__ for m in pmetrics] # type: ignore res = res.dropna(subset=mtr_cols, how="all") # TODO: ok to remove empty? - res = self._append_xy_to_res(res, cc) - res = cc._add_as_col_if_not_in_index(df, skilldf=res) # type: ignore + res = self._append_xy_to_res(res, self) + res = self._add_as_col_if_not_in_index(df, skilldf=res) # type: ignore return SkillTable(res) def _to_long_dataframe( @@ -674,30 +630,12 @@ def gridded_skill( * y (y) float64 51.5 52.5 53.5 54.5 55.5 56.5 """ - model, start, end, area = _get_deprecated_args(kwargs) # type: ignore - observation, variable = _get_deprecated_obs_var_args(kwargs) # type: ignore - assert kwargs == {}, f"Unknown keyword arguments: {kwargs}" - - cmp = self.sel( - model=model, - observation=observation, - quantity=variable, - start=start, - end=end, - area=area, - ) - - if cmp.n_points == 0: - raise ValueError("Dataset is empty, no data to compare.") - - ## ---- end of deprecated code ---- - metrics = _parse_metric(metrics) - df = cmp._to_long_dataframe() + df = self._to_long_dataframe() df = _add_spatial_grid_to_df(df=df, bins=bins, binsize=binsize) - agg_cols = _parse_groupby(by, n_mod=cmp.n_models, n_qnt=cmp.n_quantities) + agg_cols = _parse_groupby(by, n_mod=self.n_models, n_qnt=self.n_quantities) if "x" not in agg_cols: agg_cols.insert(0, "x") if "y" not in agg_cols: @@ -764,39 +702,21 @@ def mean_skill( >>> sk = cc.mean_skill(weights={"EPL": 2.0}) # more weight on EPL, others=1.0 """ - # TODO remove in v1.1 - model, start, end, area = _get_deprecated_args(kwargs) # type: ignore - observation, variable = _get_deprecated_obs_var_args(kwargs) # type: ignore - assert kwargs == {}, f"Unknown keyword arguments: {kwargs}" - - # filter data - cc = self.sel( - model=model, # deprecated - observation=observation, # deprecated - quantity=variable, # deprecated - start=start, # deprecated - end=end, # deprecated - area=area, # deprecated - ) - if cc.n_points == 0: - raise ValueError("Dataset is empty, no data to compare.") - - ## ---- end of deprecated code ---- - - df = cc._to_long_dataframe() # TODO: remove - mod_names = cc.mod_names + df = self._to_long_dataframe() # TODO: remove + mod_names = self.mod_names # obs_names = cmp.obs_names # df.observation.unique() - qnt_names = cc.quantity_names + qnt_names = self.quantity_names # skill assessment pmetrics = _parse_metric(metrics) - sk = cc.skill(metrics=pmetrics) + sk = self.skill(metrics=pmetrics) if sk is None: + # TODO don't return None return None skilldf = sk.to_dataframe() # weights - weights = cc._parse_weights(weights, sk.obs_names) + weights = self._parse_weights(weights, sk.obs_names) skilldf["weights"] = ( skilldf.n if weights is None else np.tile(weights, len(mod_names)) # type: ignore ) @@ -805,7 +725,7 @@ def weighted_mean(x: Any) -> Any: return np.average(x, weights=skilldf.loc[x.index, "weights"]) # group by - by = cc._mean_skill_by(skilldf, mod_names, qnt_names) # type: ignore + by = self._mean_skill_by(skilldf, mod_names, qnt_names) # type: ignore agg = {"n": "sum"} for metric in pmetrics: # type: ignore agg[metric.__name__] = weighted_mean # type: ignore @@ -815,7 +735,7 @@ def weighted_mean(x: Any) -> Any: res.index.name = "model" # output - res = cc._add_as_col_if_not_in_index(df, res, fields=["model", "quantity"]) # type: ignore + res = self._add_as_col_if_not_in_index(df, res, fields=["model", "quantity"]) # type: ignore return SkillTable(res.astype({"n": int})) # def mean_skill_points( @@ -1004,32 +924,7 @@ def score( if not (callable(metric) or isinstance(metric, str)): raise ValueError("metric must be a string or a function") - model, start, end, area = _get_deprecated_args(kwargs) # type: ignore - observation, variable = _get_deprecated_obs_var_args(kwargs) # type: ignore - assert kwargs == {}, f"Unknown keyword arguments: {kwargs}" - - if model is None: - models = self.mod_names - else: - # TODO: these two lines looks familiar, extract to function - models = [model] if np.isscalar(model) else model # type: ignore - models = [_get_name(m, self.mod_names) for m in models] # type: ignore - - cmp = self.sel( - model=models, # deprecated - observation=observation, # deprecated - quantity=variable, # deprecated - start=start, # deprecated - end=end, # deprecated - area=area, # deprecated - ) - - if cmp.n_points == 0: - raise ValueError("Dataset is empty, no data to compare.") - - ## ---- end of deprecated code ---- - - sk = cmp.mean_skill(weights=weights, metrics=[metric]) + sk = self.mean_skill(weights=weights, metrics=[metric]) df = sk.to_dataframe() metric_name = metric if isinstance(metric, str) else metric.__name__ @@ -1153,29 +1048,7 @@ def scatter( ): warnings.warn("scatter is deprecated, use plot.scatter instead", FutureWarning) - # TODO remove in v1.1 - model, start, end, area = _get_deprecated_args(kwargs) - observation, variable = _get_deprecated_obs_var_args(kwargs) - - # select model - mod_idx = _get_idx(model, self.mod_names) - mod_name = self.mod_names[mod_idx] - - # select variable - qnt_idx = _get_idx(variable, self.quantity_names) - qnt_name = self.quantity_names[qnt_idx] - - # filter data - cmp = self.sel( - model=mod_name, - observation=observation, - quantity=qnt_name, - start=start, - end=end, - area=area, - ) - - return cmp.plot.scatter( + return self.plot.scatter( bins=bins, quantiles=quantiles, fit_to_quantiles=fit_to_quantiles, @@ -1206,30 +1079,13 @@ def taylor( ): warnings.warn("taylor is deprecated, use plot.taylor instead", FutureWarning) - model, start, end, area = _get_deprecated_args(kwargs) - observation, variable = _get_deprecated_obs_var_args(kwargs) - assert kwargs == {}, f"Unknown keyword arguments: {kwargs}" - - cmp = self.sel( - model=model, - observation=observation, - quantity=variable, - start=start, - end=end, - area=area, - ) - - if cmp.n_points == 0: - warnings.warn("No data!") - return - if (not aggregate_observations) and (not normalize_std): raise ValueError( "aggregate_observations=False is only possible if normalize_std=True!" ) metrics = [mtr._std_obs, mtr._std_mod, mtr.cc] - skill_func = cmp.mean_skill if aggregate_observations else cmp.skill + skill_func = self.mean_skill if aggregate_observations else self.skill sk = skill_func(metrics=metrics) df = sk.to_dataframe() diff --git a/modelskill/comparison/_comparison.py b/modelskill/comparison/_comparison.py index 1545f033c..88944f51f 100644 --- a/modelskill/comparison/_comparison.py +++ b/modelskill/comparison/_comparison.py @@ -56,7 +56,6 @@ def skill( self, by: str | Iterable[str] | None = None, metrics: Iterable[str] | Iterable[Callable] | str | Callable | None = None, - **kwargs: Any, ) -> SkillTable: ... def gridded_skill( @@ -66,7 +65,6 @@ def gridded_skill( by: str | Iterable[str] | None = None, metrics: Iterable[str] | Iterable[Callable] | str | Callable | None = None, n_min: int | None = None, - **kwargs: Any, ) -> SkillGrid: ... @@ -149,49 +147,6 @@ def _is_model(da: xr.DataArray) -> bool: return str(da.attrs["kind"]) == "model" -# TODO remove in v1.1 -def _get_deprecated_args(kwargs): # type: ignore - model, start, end, area = None, None, None, None - - # Don't bother refactoring this, it will be removed in v1.1 - if "model" in kwargs: - model = kwargs.pop("model") - if model is not None: - warnings.warn( - f"The 'model' argument is deprecated, use 'sel(model='{model}')' instead", - FutureWarning, - ) - - if "start" in kwargs: - start = kwargs.pop("start") - - if start is not None: - warnings.warn( - f"The 'start' argument is deprecated, use 'sel(start={start})' instead", - FutureWarning, - ) - - if "end" in kwargs: - end = kwargs.pop("end") - - if end is not None: - warnings.warn( - f"The 'end' argument is deprecated, use 'sel(end={end})' instead", - FutureWarning, - ) - - if "area" in kwargs: - area = kwargs.pop("area") - - if area is not None: - warnings.warn( - f"The 'area' argument is deprecated, use 'sel(area={area})' instead", - FutureWarning, - ) - - return model, start, end, area - - def _validate_metrics(metrics: Iterable[Any]) -> None: for m in metrics: if isinstance(m, str): @@ -986,7 +941,6 @@ def skill( self, by: str | Iterable[str] | None = None, metrics: Iterable[str] | Iterable[Callable] | str | Callable | None = None, - **kwargs: Any, ) -> SkillTable: """Skill assessment of model(s) @@ -1031,26 +985,12 @@ def skill( """ metrics = _parse_metric(metrics, directional=self.quantity.is_directional) - # TODO remove in v1.1 - model, start, end, area = _get_deprecated_args(kwargs) # type: ignore - if kwargs != {}: - raise AttributeError(f"Unknown keyword arguments: {kwargs}") - - cmp = self.sel( - model=model, - start=start, - end=end, - area=area, - ) - if cmp.n_points == 0: - raise ValueError("No data selected for skill assessment") - - by = _parse_groupby(by, n_mod=cmp.n_models, n_qnt=1) + by = _parse_groupby(by, n_mod=self.n_models, n_qnt=1) - df = cmp._to_long_dataframe() + df = self._to_long_dataframe() res = _groupby_df(df, by=by, metrics=metrics) - res["x"] = np.nan if self.gtype == "track" else cmp.x - res["y"] = np.nan if self.gtype == "track" else cmp.y + res["x"] = np.nan if self.gtype == "track" else self.x + res["y"] = np.nan if self.gtype == "track" else self.y res = self._add_as_col_if_not_in_index(df, skilldf=res) return SkillTable(res) @@ -1105,17 +1045,9 @@ def score( if not (callable(metric) or isinstance(metric, str)): raise ValueError("metric must be a string or a function") - # TODO remove in v1.1 - model, start, end, area = _get_deprecated_args(kwargs) # type: ignore - assert kwargs == {}, f"Unknown keyword arguments: {kwargs}" - sk = self.skill( by=["model", "observation"], metrics=[metric], - model=model, # deprecated - start=start, # deprecated - end=end, # deprecated - area=area, # deprecated ) df = sk.to_dataframe() @@ -1131,7 +1063,6 @@ def gridded_skill( by: str | Iterable[str] | None = None, metrics: Iterable[str] | Iterable[Callable] | str | Callable | None = None, n_min: int | None = None, - **kwargs: Any, ): """Aggregated spatial skill assessment of model(s) on a regular spatial grid. @@ -1188,25 +1119,14 @@ def gridded_skill( * y (y) float64 51.5 52.5 53.5 54.5 55.5 56.5 """ - # TODO remove in v1.1 - model, start, end, area = _get_deprecated_args(kwargs) - assert kwargs == {}, f"Unknown keyword arguments: {kwargs}" - - cmp = self.sel( - model=model, - start=start, - end=end, - area=area, - ) - metrics = _parse_metric(metrics) - if cmp.n_points == 0: + if self.n_points == 0: raise ValueError("No data to compare") - df = cmp._to_long_dataframe() + df = self._to_long_dataframe() df = _add_spatial_grid_to_df(df=df, bins=bins, binsize=binsize) - agg_cols = _parse_groupby(by=by, n_mod=cmp.n_models, n_qnt=1) + agg_cols = _parse_groupby(by=by, n_mod=self.n_models, n_qnt=1) if "x" not in agg_cols: agg_cols.insert(0, "x") if "y" not in agg_cols: @@ -1402,16 +1322,7 @@ def scatter( "This method is deprecated, use plot.scatter instead", FutureWarning ) - # TODO remove in v1.1 - model, start, end, area = _get_deprecated_args(kwargs) - - # self.plot.scatter( - self.sel( - model=model, - start=start, - end=end, - area=area, - ).plot.scatter( + self.plot.scatter( bins=bins, quantiles=quantiles, fit_to_quantiles=fit_to_quantiles, diff --git a/modelskill/skill.py b/modelskill/skill.py index 37442f6be..3c4fc6d18 100644 --- a/modelskill/skill.py +++ b/modelskill/skill.py @@ -312,7 +312,7 @@ class SkillArray: def __init__(self, data: pd.DataFrame) -> None: self.data = data self._ser = data.iloc[:, -1] # last column is the metric - + self.plot = SkillArrayPlotter(self) """Plot using the SkillArrayPlotter @@ -435,8 +435,6 @@ def __init__(self, data: pd.DataFrame): self.data: pd.DataFrame = ( data if isinstance(data, pd.DataFrame) else data.to_dataframe() ) - # TODO remove in v1.1 - self.plot = DeprecatedSkillPlotter(self) # type: ignore # TODO: remove? @property @@ -511,12 +509,10 @@ def _repr_html_(self) -> Any: return self._df._repr_html_() @overload - def __getitem__(self, key: Hashable | int) -> SkillArray: - ... + def __getitem__(self, key: Hashable | int) -> SkillArray: ... @overload - def __getitem__(self, key: Iterable[Hashable]) -> SkillTable: - ... + def __getitem__(self, key: Iterable[Hashable]) -> SkillTable: ... def __getitem__( self, key: Hashable | Iterable[Hashable] @@ -559,7 +555,7 @@ def iloc(self, *args, **kwargs): # type: ignore @property def loc(self, *args, **kwargs): # type: ignore - return self.data.loc(*args, **kwargs) + return self.data.loc(*args, **kwargs) def sort_index(self, *args, **kwargs) -> SkillTable: # type: ignore """Sort by index (level) e.g. sorting by observation diff --git a/tests/test_aggregated_skill.py b/tests/test_aggregated_skill.py index 74fc2e181..ea221be1c 100644 --- a/tests/test_aggregated_skill.py +++ b/tests/test_aggregated_skill.py @@ -128,13 +128,6 @@ def test_skill(cc1): assert "bias" in repr(sk) -def test_skill_bad_args(cc1): - - # is there a better error type? - with pytest.raises(AttributeError): - cc1.skill(nonexisting_arg=1) - - def test_skill_multi_model(cc2): sk = cc2.skill(metrics=["rmse", "bias"]) diff --git a/tests/test_multimodelcompare.py b/tests/test_multimodelcompare.py index 6fa0cdb0c..62110b316 100644 --- a/tests/test_multimodelcompare.py +++ b/tests/test_multimodelcompare.py @@ -83,14 +83,6 @@ def test_mm_skill(cc): assert df.iloc[3].name[1] == "HKNA" assert pytest.approx(df.iloc[3].mae, 1e-5) == 0.214476 - # TODO remove in v1.1 - with pytest.warns(FutureWarning): - df = cc.skill(start="2017-10-27 00:01").to_dataframe() - - assert df.iloc[3].name[0] == "SW_2" - assert df.iloc[3].name[1] == "HKNA" - assert pytest.approx(df.iloc[3].mae, 1e-5) == 0.214476 - def test_mm_skill_model(cc): df = cc.sel(model="SW_1").skill().to_dataframe() @@ -113,11 +105,6 @@ def test_mm_sel_missing_model(cc): def test_mm_skill_obs(cc): - with pytest.warns(FutureWarning): - sk = cc.skill(observation="c2") - assert len(sk) == 2 - assert pytest.approx(sk.loc["SW_2"].bias) == 0.081431053 - sk = cc.sel(observation="c2").skill() assert len(sk) == 2 assert pytest.approx(sk.loc["SW_2"].bias) == 0.081431053 @@ -296,15 +283,6 @@ def test_mm_mean_skill_weights_dict(cc): def test_mm_scatter(cc): - with pytest.warns(FutureWarning): - cc.sel(model="SW_2").scatter(start="2017-10-28") - - with pytest.warns(FutureWarning): - cc.sel(model="SW_2")[0].scatter(start="2017-10-28") - - with pytest.warns(FutureWarning): - cc.sel(model="SW_2").scatter() - # scatter is the default plot ax = cc.sel(model="SW_2").plot() assert "SW_2" in ax.get_title()