diff --git a/changelog.d/remove-public-api-functions.removed.md b/changelog.d/remove-public-api-functions.removed.md new file mode 100644 index 000000000..5755fa790 --- /dev/null +++ b/changelog.d/remove-public-api-functions.removed.md @@ -0,0 +1 @@ +Removed `Reform.from_api`, `Reform.api_id` and `policyengine_core.charts.api` (with the chart functions `get_api_chart_data`, `intra_decile_chart` and `decile_chart` that `policyengine_core.charts` exported from it), which sent HTTP requests to the PolicyEngine Public API; build reforms with `Reform.from_dict` instead. diff --git a/docs/usage/reforms.ipynb b/docs/usage/reforms.ipynb index b9bd4c2ea..c969559a0 100644 --- a/docs/usage/reforms.ipynb +++ b/docs/usage/reforms.ipynb @@ -25,7 +25,7 @@ "```python\n", "Reform.from_dict(\n", " parameter_values: dict, # Parameter path -> period -> value mappings\n", - " country_id: str = None, # Optional: country code for API integration\n", + " country_id: str = None, # Optional: the country the reform applies to\n", " name: str = None, # Optional: human-readable name for the reform\n", ") -> Reform\n", "```" @@ -197,9 +197,9 @@ "cell_type": "markdown", "metadata": {}, "source": [ - "### Named reforms for API integration\n", + "### Named reforms\n", "\n", - "When working with the PolicyEngine API, you can provide a `country_id` and `name` for better integration." + "You can provide a `country_id` and a `name`, which are stored on the reform class to identify it." ] }, { @@ -208,7 +208,7 @@ "metadata": {}, "outputs": [], "source": [ - "# Named reform with country ID (useful for API integration)\n", + "# Named reform with a country ID\n", "named_reform = Reform.from_dict(\n", " {\n", " \"benefits.basic_income\": {\n", diff --git a/policyengine_core/charts/__init__.py b/policyengine_core/charts/__init__.py index f12564970..a488cffa9 100644 --- a/policyengine_core/charts/__init__.py +++ b/policyengine_core/charts/__init__.py @@ -12,4 +12,3 @@ ) from .bar import * -from .api import * diff --git a/policyengine_core/charts/api.py b/policyengine_core/charts/api.py deleted file mode 100644 index 4bcd6736c..000000000 --- a/policyengine_core/charts/api.py +++ /dev/null @@ -1,226 +0,0 @@ -import requests -import time -import plotly.graph_objects as go -from .formatting import ( - DARK_GRAY, - MEDIUM_DARK_GRAY, - WHITE, - BLUE_LIGHT, - BLUE_PRIMARY, - format_fig, -) - - -def get_api_chart_data( - country_id: str, - reform_policy_id: int, - chart_key: str, - region: str, - time_period: str, - baseline_policy_id: int = None, - version: str = None, -) -> dict: - if baseline_policy_id is None or version is None: - response = requests.get( - f"https://api.policyengine.org/{country_id}/metadata", - # Add timeouts so stalled API responses can't hang us forever - # (bug M11). - timeout=30, - ) - result = response.json().get("result", {}) - baseline_policy_id = result.get("current_law_id") - version = result.get("version") - url = f"https://api.policyengine.org/{country_id}/economy/{reform_policy_id}/over/{baseline_policy_id}?region={region}&time_period={time_period}&version={version}" - response = requests.get(url, timeout=60) - if not response.ok: - raise ValueError(response.text) - json = response.json() - while json.get("status") == "computing": - time.sleep(1) - response = requests.get(url, timeout=60) - if not response.ok: - raise ValueError(response.text) - json = response.json() - return json.get("result", {}).get(chart_key) - - -# Specific chart definitions - - -def intra_decile_chart( - country_id: str, - reform_policy_id: int, - region: str, - time_period: str, - baseline_policy_id: int = None, -) -> go.Figure: - impact = get_api_chart_data( - country_id=country_id, - reform_policy_id=reform_policy_id, - chart_key="intra_decile", - baseline_policy_id=baseline_policy_id, - region=region, - time_period=time_period, - ) - impact = {"intra_decile": impact} - - decile_numbers = list(range(1, 11)) - outcome_labels = [ - "Gain more than 5%", - "Gain less than 5%", - "No change", - "Lose less than 5%", - "Lose more than 5%", - ] - outcome_colours = [ - DARK_GRAY, - MEDIUM_DARK_GRAY, - WHITE, - BLUE_LIGHT, - BLUE_PRIMARY, - ][::-1] - - data = [] - - for outcome_label, outcome_colour in zip(outcome_labels, outcome_colours): - data.append( - { - "type": "bar", - "y": ["All"], - "x": [impact["intra_decile"]["all"][outcome_label]], - "name": outcome_label, - "legendgroup": outcome_label, - "offsetgroup": outcome_label, - "marker": { - "color": outcome_colour, - }, - "orientation": "h", - "text": [f"{impact['intra_decile']['all'][outcome_label] * 100:.0f}%"], - "textposition": "inside", - "textangle": 0, - "xaxis": "x", - "yaxis": "y", - "showlegend": False, - "hoverinfo": "none", - } - ) - - for outcome_label, outcome_colour in zip(outcome_labels, outcome_colours): - data.append( - { - "type": "bar", - "y": decile_numbers, - "x": impact["intra_decile"]["deciles"][outcome_label], - "name": outcome_label, - "marker": { - "color": outcome_colour, - }, - "orientation": "h", - "text": [ - f"{value * 100:.0f}%" - for value in impact["intra_decile"]["deciles"][outcome_label] - ], - "textposition": "inside", - "textangle": 0, - "xaxis": "x2", - "yaxis": "y2", - "hoverinfo": "none", - } - ) - - layout = { - "barmode": "stack", - "grid": { - "rows": 2, - "columns": 1, - }, - "yaxis": { - "title": "", - "tickvals": ["All"], - "domain": [0.91, 1], - }, - "xaxis": { - "title": "", - "tickformat": ".0%", - "anchor": "y", - "matches": "x2", - "showgrid": False, - "showticklabels": False, - }, - "xaxis2": { - "title": "Population share", - "tickformat": ".0%", - "anchor": "y2", - }, - "yaxis2": { - "title": "Income decile", - "tickvals": decile_numbers, - "anchor": "x2", - "domain": [0, 0.85], - }, - "uniformtext": { - "mode": "hide", - "minsize": 10, - }, - } - - return format_fig(go.Figure(data=data, layout=layout)).update_traces( - marker_line_width=0, - width=[0.9] * 10, - ) - - -def decile_chart( - country_id: str, - reform_policy_id: int, - region: str, - time_period: str, - baseline_policy_id: int = None, -) -> go.Figure: - impact = get_api_chart_data( - country_id=country_id, - reform_policy_id=reform_policy_id, - chart_key="decile", - baseline_policy_id=baseline_policy_id, - region=region, - time_period=time_period, - ) - impact = {"decile": impact} - - decile_numbers = list(range(1, 11)) - # Sort deciles by key order 1 to 10 - decile_values = [] - for i in decile_numbers: - decile_values.append(impact["decile"]["relative"][str(i)]) - - data = [ - { - "x": decile_numbers, - "y": decile_values, - "type": "bar", - "marker": { - "color": [ - DARK_GRAY if value < 0 else BLUE_PRIMARY for value in decile_values - ], - }, - "text": [f"{value:+.1%}" for value in decile_values], - "textangle": 0, - } - ] - - layout = { - "xaxis": { - "title": "Income decile", - "tickvals": decile_numbers, - }, - "yaxis": { - "title": "Relative change", - "tickformat": "+,.0%", - }, - "uniformtext": { - "mode": "hide", - "minsize": 8, - }, - } - - return format_fig(go.Figure(data=data, layout=layout)) diff --git a/policyengine_core/reforms/reform.py b/policyengine_core/reforms/reform.py index 9358ee5c1..afcee0358 100644 --- a/policyengine_core/reforms/reform.py +++ b/policyengine_core/reforms/reform.py @@ -14,17 +14,6 @@ Period, ) -import requests - - -class classproperty(object): - def __init__(self, f): - self.f = f - - def __get__(self, obj, owner): - return self.f(owner) - - # --------------------------------------------------------------------------- # Reform period-key handling. # @@ -88,23 +77,6 @@ def _update_start_instant(update_kwargs: dict): return instant_("0001-01-01") -def _api_period_range(start: str, stop: str) -> str: - """Clamp an API policy's ``[start, stop]`` to the supported window and - return an explicit ``"start.stop"`` key, so :meth:`Reform.from_dict` - treats every API policy as bounded. (A single-civil-year intersection - would otherwise stringify to a bare ``"YYYY"`` which, under the - from-onward rule, would extend the change past the policy's end.)""" - clamped = period_(_SCALAR_REFORM_PERIOD).intersection( - instant_(start), instant_(stop) - ) - if clamped is None: - raise ValueError( - f"Policy period '{start}.{stop}' does not overlap the supported " - f"reform window ({_SCALAR_REFORM_PERIOD})." - ) - return f"{clamped.start}.{clamped.stop}" - - class Reform(TaxBenefitSystem): """A modified TaxBenefitSystem @@ -138,10 +110,10 @@ class Reform(TaxBenefitSystem): """The name of the reform. This is used to identify the reform in the UI.""" country_id: str = None - """The country id of the reform. This is used to inform any calls to the PolicyEngine API.""" + """The country id of the reform, if one was given.""" parameter_values: dict = None - """The parameter values of the reform. This is used to inform any calls to the PolicyEngine API.""" + """The parameter values a reform created by :meth:`Reform.from_dict` applies.""" simulation: "Simulation" = None @@ -281,80 +253,6 @@ def apply(self): return reform - @staticmethod - def from_api( - api_id: str, - country_id: str = None, - ) -> Reform: - """Create a reform from a dictionary of parameters. - - Args: - parameters: A dictionary of parameter -> { period -> value } pairs. - - Returns: - A reform. - """ - - data = requests.get( - f"https://api.policyengine.org/{country_id}/policy/{api_id}", - # Timeout so a stalled API can't hang the caller forever - # (bug M11). - timeout=30, - ).json() - - parameter_values = data.get("result", {}).get("policy_json", {}) - - for path in parameter_values: - keys_to_remove = [] - for start_stop_str in list(parameter_values[path].keys()): - start, stop = start_stop_str.split(".") - time_period = _api_period_range(start, stop) - parameter_values[path][time_period] = parameter_values[path][ - start_stop_str - ] - keys_to_remove.append(start_stop_str) - for key in keys_to_remove: - del parameter_values[path][key] - - return Reform.from_dict( - parameter_values, - country_id, - data.get("result", {}).get("label", None), - ) - - @classproperty - def api_id(self): - if self.country_id is None: - raise ValueError( - "`country_id` is not set. This is required to use the API." - ) - if self.parameter_values is None: - raise ValueError( - "`parameter_values` is not set. This is required to use the API." - ) - - sanitised_parameter_values = {} - - for path, period_values in self.parameter_values.items(): - sanitised_period_values = {} - for period, value in period_values.items(): - period = period_(period) - sanitised_period_values[f"{period.start}.{period.stop}"] = value - sanitised_parameter_values[path] = sanitised_period_values - - response = requests.post( - f"https://api.policyengine.org/{self.country_id}/policy", - json={ - "data": sanitised_parameter_values, - "name": self.name, - }, - # Timeout so a stalled API can't hang the caller forever - # (bug M11). - timeout=30, - ) - - return response.json().get("result", {}).get("policy_id") - def set_parameter( path: Union[Parameter, str], diff --git a/tests/core/test_reform_period_keys.py b/tests/core/test_reform_period_keys.py index edff5ea6b..816cad0a8 100644 --- a/tests/core/test_reform_period_keys.py +++ b/tests/core/test_reform_period_keys.py @@ -4,7 +4,6 @@ from policyengine_core.periods import Instant, instant, period from policyengine_core.reforms.reform import ( - _api_period_range, _bounded_period_update_kwargs, _eternity_update_kwargs, _from_instant_update_kwargs, @@ -104,19 +103,3 @@ def test_period_key_update_kwargs_malformed_raises(): ) def test_update_start_instant(update_kwargs, expected): assert _update_start_instant(update_kwargs) == expected - - -# ---- from_api range normalization ----------------------------------------- - - -def test_api_period_range_single_year_stays_bounded(): - assert _api_period_range("2026-01-01", "2026-12-31") == "2026-01-01.2026-12-31" - - -def test_api_period_range_multi_year_stays_bounded(): - assert _api_period_range("2026-01-01", "2030-12-31") == "2026-01-01.2030-12-31" - - -def test_api_period_range_out_of_window_raises(): - with pytest.raises(ValueError): - _api_period_range("2200-01-01", "2300-01-01")