Skip to content

Commit

Permalink
Converted print statements to errors/warnings; updated tests
Browse files Browse the repository at this point in the history
  • Loading branch information
CBROWN-ONS committed Sep 18, 2023
1 parent 87216ed commit a53a340
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 37 deletions.
21 changes: 17 additions & 4 deletions src/transport_performance/gtfs/validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand All @@ -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(
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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"
Expand Down
57 changes: 24 additions & 33 deletions tests/gtfs/test_validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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):
Expand Down Expand Up @@ -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(
Expand All @@ -182,13 +179,25 @@ def test_viz_stops_defence(self, mocked_print, tmpdir, gtfs_fixture):
match="`geom_crs`.*string or integer. Found <class 'float'>",
):
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):
Expand All @@ -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."""
Expand Down

0 comments on commit a53a340

Please sign in to comment.