From d9da952ae3f49f0fce0363d0678e00aaee712c00 Mon Sep 17 00:00:00 2001 From: r-leyshon Date: Mon, 11 Sep 2023 11:57:28 +0100 Subject: [PATCH] refactor: Several type utils -> _type_defence --- src/transport_performance/gtfs/gtfs_utils.py | 7 +- .../gtfs/report/report_utils.py | 9 ++- src/transport_performance/gtfs/routes.py | 4 +- src/transport_performance/gtfs/validation.py | 33 ++++----- src/transport_performance/osm/osm_utils.py | 4 +- src/transport_performance/utils/defence.py | 68 ++++++------------- tests/gtfs/test_routes.py | 6 +- tests/osm/test_osm_utils.py | 5 +- 8 files changed, 51 insertions(+), 85 deletions(-) diff --git a/src/transport_performance/gtfs/gtfs_utils.py b/src/transport_performance/gtfs/gtfs_utils.py index e3bede01..a78c3940 100644 --- a/src/transport_performance/gtfs/gtfs_utils.py +++ b/src/transport_performance/gtfs/gtfs_utils.py @@ -9,8 +9,7 @@ from transport_performance.utils.defence import ( _is_expected_filetype, _check_list, - _dataframe_defence, - _bool_defence, + _type_defence, ) @@ -102,8 +101,8 @@ def convert_pandas_to_plotly( } } # defences - _dataframe_defence(df, "df") - _bool_defence(return_html, "return_html") + _type_defence(df, "df", pd.DataFrame) + _type_defence(return_html, "return_html", bool) if scheme not in list(schemes.keys()): raise LookupError( f"{scheme} is not a valid colour scheme." diff --git a/src/transport_performance/gtfs/report/report_utils.py b/src/transport_performance/gtfs/report/report_utils.py index ccb6de10..96aa46cd 100644 --- a/src/transport_performance/gtfs/report/report_utils.py +++ b/src/transport_performance/gtfs/report/report_utils.py @@ -5,8 +5,7 @@ import os from transport_performance.utils.defence import ( - _bool_defence, - _string_defence, + _type_defence, _handle_path_like, _check_parent_dir_exists, ) @@ -92,9 +91,9 @@ def insert( None """ - _string_defence(placeholder, "placeholder") - _string_defence(value, "value") - _bool_defence(replace_multiple, "replace_multiple") + _type_defence(placeholder, "placeholder", str) + _type_defence(value, "value", str) + _type_defence(replace_multiple, "replace_multiple", bool) occurences = len(self.template.split(f"[{placeholder}]")) - 1 if occurences > 1 and not replace_multiple: raise ValueError( diff --git a/src/transport_performance/gtfs/routes.py b/src/transport_performance/gtfs/routes.py index e989645f..d0003f57 100644 --- a/src/transport_performance/gtfs/routes.py +++ b/src/transport_performance/gtfs/routes.py @@ -4,7 +4,7 @@ import requests import warnings -from transport_performance.utils.defence import _url_defence, _bool_defence +from transport_performance.utils.defence import _url_defence, _type_defence warnings.filterwarnings( action="ignore", category=DeprecationWarning, module=".*pkg_resources" @@ -98,7 +98,7 @@ def scrape_route_type_lookup( for url in [gtfs_url, ext_spec_url]: _url_defence(url) - _bool_defence(extended_schema, "extended_schema") + _type_defence(extended_schema, "extended_schema", bool) # Get the basic scheme lookup resp_txt = _get_response_text(gtfs_url) soup = BeautifulSoup(resp_txt, "html.parser") diff --git a/src/transport_performance/gtfs/validation.py b/src/transport_performance/gtfs/validation.py index a8483ac9..523ea305 100644 --- a/src/transport_performance/gtfs/validation.py +++ b/src/transport_performance/gtfs/validation.py @@ -23,12 +23,7 @@ _check_namespace_export, _check_parent_dir_exists, _check_column_in_df, - _bool_defence, - _integer_defence, - _string_defence, - _dataframe_defence, - _dict_defence, - _string_and_nonetype_defence, + _type_defence, ) from transport_performance.gtfs.report.report_utils import ( @@ -733,17 +728,17 @@ def _plot_summary( """ # parameter type defences - _dataframe_defence(summary_df, "summary_df") - _string_defence(day_column, "day_column") - _string_defence(target_column, "target_column") - _dict_defence(plotly_kwargs, "plotly_kwargs") - _bool_defence(return_html, "return_html") - _integer_defence(width, "width") - _integer_defence(height, "height") - _string_and_nonetype_defence(xlabel, "xlabel") - _string_and_nonetype_defence(ylabel, "ylabel") - _bool_defence(save_html, "save_html") - _bool_defence(save_image, "save_iamge") + _type_defence(summary_df, "summary_df", pd.DataFrame) + _type_defence(day_column, "day_column", str) + _type_defence(target_column, "target_column", str) + _type_defence(plotly_kwargs, "plotly_kwargs", dict) + _type_defence(return_html, "return_html", bool) + _type_defence(width, "width", int) + _type_defence(height, "height", int) + _type_defence(xlabel, "xlabel", (str, type(None))) + _type_defence(ylabel, "ylabel", (str, type(None))) + _type_defence(save_html, "save_html", bool) + _type_defence(save_image, "save_iamge", bool) _check_parent_dir_exists(save_pth, "save_pth", create=True) # orientation input defences @@ -1241,8 +1236,8 @@ def html_report( None """ - _bool_defence(overwrite, "overwrite") - _string_defence(summary_type, "summary_type") + _type_defence(overwrite, "overwrite", bool) + _type_defence(summary_type, "summary_type", str) set_up_report_dir(path=report_dir, overwrite=overwrite) summary_type = summary_type.lower() if summary_type not in ["mean", "min", "max", "median"]: diff --git a/src/transport_performance/osm/osm_utils.py b/src/transport_performance/osm/osm_utils.py index e4501416..d086eb6a 100644 --- a/src/transport_performance/osm/osm_utils.py +++ b/src/transport_performance/osm/osm_utils.py @@ -3,7 +3,7 @@ from pyprojroot import here from transport_performance.utils.defence import ( - _bool_defence, + _type_defence, _check_list, _check_parent_dir_exists, _is_expected_filetype, @@ -54,7 +54,7 @@ def filter_osm( "tag_filter": tag_filter, "install_osmosis": install_osmosis, }.items(): - _bool_defence(val, param_nm=nm) + _type_defence(val, nm, bool) # check bbox values makes sense, else osmosis will error if not bbox[0] < bbox[2]: raise ValueError( diff --git a/src/transport_performance/utils/defence.py b/src/transport_performance/utils/defence.py index 8567199f..123fa729 100644 --- a/src/transport_performance/utils/defence.py +++ b/src/transport_performance/utils/defence.py @@ -150,68 +150,40 @@ def _check_namespace_export(pkg=np, func=np.min): def _url_defence(url): """Defence checking. Not exported.""" - if not isinstance(url, str): - raise TypeError(f"url {url} expected string, instead got {type(url)}") - elif not url.startswith((r"http://", r"https://")): + _type_defence(url, "url", str) + if not url.startswith((r"http://", r"https://")): raise ValueError(f"url string expected protocol, instead found {url}") return None -def _bool_defence(some_bool, param_nm): - """Defence checking. Not exported.""" - if not isinstance(some_bool, bool): - raise TypeError( - f"`{param_nm}` expected boolean. Got {type(some_bool)}" - ) - - return None - - -def _string_defence(string, param_nm): - """Defence checking. Not exported.""" - if not isinstance(string, str): - raise TypeError(f"'{param_nm}' expected str. Got {type(string)}") - - return None - - -def _integer_defence(some_int, param_nm): - """Defence checking. Not exported.""" - if not isinstance(some_int, int): - raise TypeError(f"'{param_nm}' expected int. Got {type(some_int)}") - - return None +def _type_defence(some_object, param_nm, types) -> None: + """Defence checking utility. Can handle NoneType. + Parameters + ---------- + some_object : Any + Object to test with isinstance. + param_nm : str + A name for the parameter. Useful when this utility is used in a wrapper + to inherit the parent's parameter name and present in error message. + types : type or tuple + A type or a tuple of types to test `some_object` against. -def _dict_defence(some_dict, param_nm): - """Defence checking. Not exported.""" - if not isinstance(some_dict, dict): - raise TypeError(f"'{param_nm}' expected dict. Got {type(some_dict)}") - - return None - + Raises + ------ + TypeError + `some_object` is not of type `types`. -def _dataframe_defence(some_df, param_nm): - """Defence checking. Not exported.""" - if not isinstance(some_df, pd.DataFrame): + """ + if not isinstance(some_object, types): raise TypeError( - f"'{param_nm}' expected pd.DataFrame. Got {type(some_df)}" + f"`{param_nm}` expected {types}. Got {type(some_object)}" ) return None -# main use case for this is to avoid -# complexity limits in -# GtfsInstance._plot_summary() -def _string_and_nonetype_defence(some_value, param_nm): - if not isinstance(some_value, (str, type(None))): - raise TypeError( - f"'{param_nm}' expected type str. Found type {type(some_value)}" - ) - - def _check_list(ls, param_nm, check_elements=True, exp_type=str): """Check a list and its elements for type. diff --git a/tests/gtfs/test_routes.py b/tests/gtfs/test_routes.py index 38eaa23a..efabfc2f 100644 --- a/tests/gtfs/test_routes.py +++ b/tests/gtfs/test_routes.py @@ -47,12 +47,12 @@ def test_defensive_exceptions(self): """Test the defensive checks raise as expected.""" with pytest.raises( TypeError, - match=r"url 1 expected string, instead got ", + match=r"`url` expected . Got ", ): scrape_route_type_lookup(gtfs_url=1) with pytest.raises( TypeError, - match=r"url False expected string, instead got ", + match=r"`url` expected . Got ", ): scrape_route_type_lookup(ext_spec_url=False) with pytest.raises( @@ -62,7 +62,7 @@ def test_defensive_exceptions(self): scrape_route_type_lookup(gtfs_url="foobar") with pytest.raises( TypeError, - match=r"`extended_schema` expected boolean. Got ", + match=r"`extended_schema` .* . Got ", ): scrape_route_type_lookup(extended_schema="True") diff --git a/tests/osm/test_osm_utils.py b/tests/osm/test_osm_utils.py index 04ce3803..17e145f2 100644 --- a/tests/osm/test_osm_utils.py +++ b/tests/osm/test_osm_utils.py @@ -30,13 +30,14 @@ def test_filter_osm_defense(self): # out_pth is not a path_like filter_osm(out_pth=False) with pytest.raises( - TypeError, match="`tag_filter` expected boolean. Got " + TypeError, + match="`tag_filter` expected . Got ", ): # check for boolean defense filter_osm(tag_filter=1) with pytest.raises( TypeError, - match="`install_osmosis` expected boolean. Got ", + match="`install_osmosis` .* . Got ", ): # check for boolean defense filter_osm(install_osmosis="False")