diff --git a/src/transport_performance/gtfs/validation.py b/src/transport_performance/gtfs/validation.py index 04652572..684aa19b 100644 --- a/src/transport_performance/gtfs/validation.py +++ b/src/transport_performance/gtfs/validation.py @@ -12,6 +12,7 @@ import plotly.io as plotly_io from pretty_html_table import build_table import zipfile +import warnings import pathlib from typing import Union from plotly.graph_objects import Figure as PlotlyFigure @@ -224,7 +225,9 @@ def print_alerts(self, alert_type="error"): elif isinstance(msgs, str): print(msgs) except KeyError: - print(f"No alerts of type {alert_type} were found.") + warnings.warn( + f"No alerts of type {alert_type} were found.", UserWarning + ) return None @@ -236,6 +239,7 @@ def clean_feed(self): # shows that shapes.txt is optional file. self.feed = self.feed.clean() except KeyError: + # TODO: Issue 74 - Improve this to clean feed when KeyError raised print("KeyError. Feed was not cleaned.") def viz_stops( @@ -273,8 +277,11 @@ def viz_stops( pre, ext = os.path.splitext(out_pth) if ext != ".html": - print(f"{ext} format not implemented. Writing to .html") - out_pth = os.path.normpath(pre + ".html") + raise ValueError( + f"{ext} format not implemented. Accepted " + "formats include ['.html'] . Try out_pth=" + f"'{pre+'.html'}" + ) # geoms defence if not isinstance(geoms, str): @@ -320,7 +327,13 @@ def viz_stops( m.fit_bounds(m.get_bounds()) m.save(out_pth) except KeyError: - print("Key Error. Map was not written.") + # KeyError inside of an except KeyError here. This is to provide + # a more detaailed error message on why a KeyError is being raised. + raise KeyError( + "The stops table has no 'stop_code' column. While " + "this is an optional field in a GTFS file, it " + "raises an error through the gtfs-kit package." + ) def _order_dataframe_by_day( self, df: pd.DataFrame, day_column_name: str = "day" diff --git a/tests/gtfs/test_validation.py b/tests/gtfs/test_validation.py index 40b9c3e8..91aaedbe 100644 --- a/tests/gtfs/test_validation.py +++ b/tests/gtfs/test_validation.py @@ -117,8 +117,7 @@ def test_is_valid(self, gtfs_fixture): found_cols == exp_cols ).all(), f"Expected columns {exp_cols}. Found: {found_cols}" - @patch("builtins.print") - def test_print_alerts_defence(self, mocked_print, gtfs_fixture): + def test_print_alerts_defence(self, gtfs_fixture): """Check defensive behaviour of print_alerts().""" with pytest.raises( AttributeError, @@ -127,11 +126,10 @@ def test_print_alerts_defence(self, mocked_print, gtfs_fixture): gtfs_fixture.print_alerts() gtfs_fixture.is_valid() - gtfs_fixture.print_alerts(alert_type="doesnt_exist") - fun_out = mocked_print.mock_calls - assert fun_out == [ - call("No alerts of type doesnt_exist were found.") - ], f"Expected a print about alert_type but found: {fun_out}" + with pytest.warns( + UserWarning, match="No alerts of type doesnt_exist were found." + ): + gtfs_fixture.print_alerts(alert_type="doesnt_exist") @patch("builtins.print") # testing print statements def test_print_alerts_single_case(self, mocked_print, gtfs_fixture): @@ -160,8 +158,7 @@ def test_print_alerts_multi_case(self, mocked_print, gtfs_fixture): call("Unrecognized column vehicle_journey_code"), ], f"Expected print statements about GTFS warnings. Found: {fun_out}" - @patch("builtins.print") - def test_viz_stops_defence(self, mocked_print, tmpdir, gtfs_fixture): + def test_viz_stops_defence(self, tmpdir, gtfs_fixture): """Check defensive behaviours of viz_stops().""" tmp = os.path.join(tmpdir, "somefile.html") with pytest.raises( @@ -182,13 +179,25 @@ def test_viz_stops_defence(self, mocked_print, tmpdir, gtfs_fixture): match="`geom_crs`.*string or integer. Found ", ): gtfs_fixture.viz_stops(out_pth=tmp, geom_crs=1.1) - # check missing stop_id results in print instead of exception + # check missing stop_id results in an informative error message gtfs_fixture.feed.stops.drop("stop_id", axis=1, inplace=True) - gtfs_fixture.viz_stops(out_pth=tmp) - fun_out = mocked_print.mock_calls - assert fun_out == [ - call("Key Error. Map was not written.") - ], f"Expected confirmation that map was not written. Found: {fun_out}" + with pytest.raises( + KeyError, + match="The stops table has no 'stop_code' column. While " + "this is an optional field in a GTFS file, it " + "raises an error through the gtfs-kit package.", + ): + gtfs_fixture.viz_stops(out_pth=tmp) + + with pytest.raises( + ValueError, + match=re.escape( + ".svg format not implemented. Accepted " + "formats include ['.html'] . Try out_pth=" + f"'{tmp+'.html'}" + ), + ): + gtfs_fixture.viz_stops(out_pth=tmp + ".svg") @patch("builtins.print") def test_viz_stops_point(self, mock_print, tmpdir, gtfs_fixture): @@ -206,24 +215,6 @@ def test_viz_stops_point(self, mock_print, tmpdir, gtfs_fixture): assert os.path.exists( no_parent_pth ), f"{no_parent_pth} was expected to exist but it was not found." - # check behaviour when not implemented fileext used - tmp1 = os.path.join(tmpdir, "points2.svg") - gtfs_fixture.viz_stops(out_pth=pathlib.Path(tmp1)) - # need to use regex for the first print statement, as tmpdir will - # change. - start_pat = re.compile(r"Creating parent directory:.*") - out = mock_print.mock_calls[0].__str__() - assert bool( - start_pat.search(out) - ), f"Print statement about directory creation expected. Found: {out}" - out_last = mock_print.mock_calls[-1] - assert out_last == call( - ".svg format not implemented. Writing to .html" - ), f"Expected print statement about .svg. Found: {out_last}" - write_pth = os.path.join(tmpdir, "points2.html") - assert os.path.exists( - write_pth - ), f"Map should have been written to {write_pth} but was not found." def test_viz_stops_hull(self, tmpdir, gtfs_fixture): """Check viz_stops behaviour when plotting hull geom."""