diff --git a/src/transport_performance/gtfs/validation.py b/src/transport_performance/gtfs/validation.py index 04652572..dedfede6 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,7 +277,9 @@ def viz_stops( pre, ext = os.path.splitext(out_pth) if ext != ".html": - print(f"{ext} format not implemented. Writing to .html") + warnings.warn( + f"{ext} format not implemented. Saving as .html", UserWarning + ) out_pth = os.path.normpath(pre + ".html") # geoms defence @@ -320,7 +326,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/src/transport_performance/utils/raster.py b/src/transport_performance/utils/raster.py index 05bf3fc2..ec3175ee 100644 --- a/src/transport_performance/utils/raster.py +++ b/src/transport_performance/utils/raster.py @@ -13,6 +13,10 @@ from rioxarray.merge import merge_arrays from rasterio.warp import Resampling +from transport_performance.utils.defence import ( + _check_parent_dir_exists, + _is_expected_filetype, +) def merge_raster_files( @@ -73,8 +77,9 @@ def merge_raster_files( """ # defend against case where the provided input dir does not exist - if not os.path.exists(input_dir): - raise FileNotFoundError(f"{input_dir} can not be found") + # add a dummy file to purely check if provided parent directory exists + dummy_path = os.path.join(input_dir, "dummy.txt") + _check_parent_dir_exists(dummy_path, "input_dir", create=False) # get tif files in directory, ensure some exist and select subset via regex tif_filepaths = glob.glob(f"{input_dir}/*.tif") @@ -101,12 +106,13 @@ def merge_raster_files( # merge the datasets together xds_merged = merge_arrays(arrays) - # make output_dir if it does not exist - if not os.path.exists(output_dir): - os.mkdir(output_dir) - # create full filepath for merged tif file and write to disk + # check expected file type and parent dir exists MERGED_DIR = os.path.join(output_dir, output_filename) + _is_expected_filetype( + MERGED_DIR, "MERGED_DIR", check_existing=False, exp_ext=".tif" + ) + _check_parent_dir_exists(MERGED_DIR, "MERGED_DIR", create=True) xds_merged.rio.to_raster(MERGED_DIR) # get boundaries of inputs and output raster @@ -146,8 +152,7 @@ def sum_resample_file( """ # defend against case where the provided input dir does not exist - if not os.path.exists(input_filepath): - raise FileNotFoundError(f"{input_filepath} can not be found") + _is_expected_filetype(input_filepath, "input_filepath", exp_ext=".tif") xds = rioxarray.open_rasterio(input_filepath, masked=True) @@ -161,7 +166,6 @@ def sum_resample_file( ) # make output_filepath's directory if it does not exist - if not os.path.exists(os.path.dirname(output_filepath)): - os.mkdir(output_filepath) + _check_parent_dir_exists(output_filepath, "output_filepath", create=True) xds_resampled.rio.to_raster(output_filepath) diff --git a/tests/gtfs/test_validation.py b/tests/gtfs/test_validation.py index 3c4c3bd7..9c7e65ac 100644 --- a/tests/gtfs/test_validation.py +++ b/tests/gtfs/test_validation.py @@ -259,8 +259,7 @@ def test_unmatched_service_id_behaviour(self, gtfs_fixture): len(new_valid[new_valid.message == "Undefined service_id"]) == 1 ), "gtfs-kit failed to identify missing service_id" - @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, @@ -269,11 +268,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): @@ -302,8 +300,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( @@ -324,13 +321,15 @@ 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) @patch("builtins.print") def test_viz_stops_point(self, mock_print, tmpdir, gtfs_fixture): @@ -350,7 +349,11 @@ def test_viz_stops_point(self, mock_print, tmpdir, gtfs_fixture): ), 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)) + with pytest.warns( + UserWarning, + match=".svg format not implemented. Saving as .html", + ): + 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:.*") @@ -358,10 +361,6 @@ def test_viz_stops_point(self, mock_print, tmpdir, gtfs_fixture): 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 diff --git a/tests/population/test_rasterpop.py b/tests/population/test_rasterpop.py index 5ae249d2..9920b569 100644 --- a/tests/population/test_rasterpop.py +++ b/tests/population/test_rasterpop.py @@ -4,9 +4,6 @@ in Newport. The data vales do not represent any real or meaningfull measurments , they are purely for use as a unit test and mimic input data used in this repo . - -Note: this suite of tests does not cover plotting methods. TODO add plotting -unit tests. See issue #43. """ import os @@ -21,6 +18,7 @@ from numpy.dtypes import Float64DType from pytest_lazyfixture import lazy_fixture from _pytest.python_api import RaisesContext +from contextlib import nullcontext as does_not_raise from transport_performance.population.rasterpop import RasterPop from transport_performance.utils.test_utils import _np_to_rioxarray @@ -508,3 +506,148 @@ def test_rasterpop_crs_conversion( assert np.array_equal( pop_gdf.within_urban_centre, xarr_1_uc[1]["within_uc"] ) + + @pytest.mark.parametrize( + "which, save_folder, save_filename", + [ + ("folium", "outputs", "folium.html"), + ("cartopy", "outputs", "cartopy.png"), + ("matplotlib", "outputs", "matplotlib.png"), + ], + ) + def test_plot_on_pass( + self, + xarr_1_fpath: str, + xarr_1_aoi: tuple, + xarr_1_uc: tuple, + tmp_path: str, + which: str, + save_folder: str, + save_filename: str, + ) -> None: + """Test plotting methods. + + Parameters + ---------- + xarr_1_fpath : str + filepath to dummy raster data + xarr_1_aoi : tuple + aoi polygon for dummy input + xarr_1_uc : tuple + urban centre polugon for dummy input + tmp_path : str + temporary path to save output within + which : str + plotting backend to use + save_folder: str + folder to save output within + save_filename : str + filename to use when saving the file within the temp directory + + """ + # create the full output path + output_path = os.path.join(tmp_path, save_folder, save_filename) + + # run raster pop and assert that the file is generated + rp = RasterPop(xarr_1_fpath) + rp.get_pop(xarr_1_aoi[0], urban_centre_bounds=xarr_1_uc[0]) + rp.plot(which=which, save=output_path) + assert os.path.exists(output_path) + + def test_plot_before_get_data(self, xarr_1_fpath: str) -> None: + """Test case where plot is called before getting data. + + Parameters + ---------- + xarr_1_fpath : str + file path to dummy data. + + """ + rp = RasterPop(xarr_1_fpath) + with pytest.raises( + NotImplementedError, + match="Unable to call `plot` without calling `get_pop`.", + ): + rp.plot() + + def test_plot_unknown_which( + self, + xarr_1_fpath: str, + xarr_1_aoi: tuple, + xarr_1_uc: tuple, + ) -> None: + """Test a plot call with an unknown backend plot type. + + Parameters + ---------- + xarr_1_fpath : str + file path to dummy data. + xarr_1_aoi : tuple + area of interest polygon for dummy data. + xarr_1_uc : tuple + urban centre polygon for dummy data. + + """ + rp = RasterPop(xarr_1_fpath) + rp.get_pop(xarr_1_aoi[0], urban_centre_bounds=xarr_1_uc[0]) + unknown_plot_backend = "unknown" + with pytest.raises( + ValueError, + match=( + f"Unrecognised value for `which` {unknown_plot_backend}. " + "Must be one of " + ), + ): + rp.plot(which=unknown_plot_backend) + + def test_plot_foliumn_no_uc( + self, + xarr_1_fpath: str, + xarr_1_aoi: tuple, + ) -> None: + """Test folium plotting backend with no urban centre. + + Unit test written to capture fix for issue 52. + + Parameters + ---------- + xarr_1_fpath : str + file path to dummy data. + xarr_1_aoi : tuple + area of interest polygon for dummy data. + + """ + rp = RasterPop(xarr_1_fpath) + rp.get_pop(xarr_1_aoi[0]) + with does_not_raise(): + rp.plot(which="folium") + + def test_plot_cartopy_unknown_attr_location( + self, + xarr_1_fpath: str, + xarr_1_aoi: tuple, + ) -> None: + """Test cartopy plotting backend with an unknown attribute location. + + Parameters + ---------- + xarr_1_fpath : str + Filepath to dummy data. + xarr_1_aoi : tuple + Area of interest for dummy data. + + """ + rp = RasterPop(xarr_1_fpath) + rp.get_pop(xarr_1_aoi[0]) + test_attr_location = "unknown" + with pytest.raises( + ValueError, + match=( + f"Unrecognised value for `attr_location` {test_attr_location}." + "Expecting one of " + ), + ): + rp.plot( + which="cartopy", + attr_location=test_attr_location, + ) diff --git a/tests/population/test_vectorpop.py b/tests/population/test_vectorpop.py new file mode 100644 index 00000000..8dda45c3 --- /dev/null +++ b/tests/population/test_vectorpop.py @@ -0,0 +1,23 @@ +"""Unit tests for transport_performance/population/vectorpop.py. + +At this stage, VectorPop has not been implemented, so the extent of this test +suite is to ensure the NotImplementedError is raised. + +TODO: add unit test for methods once VectorPop has been implemented. +""" + +import pytest + +from transport_performance.population.vectorpop import VectorPop + + +class TestVectorPop: + """A class to test VectorPop methods.""" + + def test_vectorpop_not_implemented(self) -> None: + """Test VectorPop raises a not implemented error.""" + with pytest.raises( + NotImplementedError, + match="This class has not yet been implemented", + ): + VectorPop() diff --git a/tests/utils/test_raster.py b/tests/utils/test_raster.py index 249ede0d..14e8f1f9 100644 --- a/tests/utils/test_raster.py +++ b/tests/utils/test_raster.py @@ -15,6 +15,9 @@ import xarray as xr import rioxarray # noqa: F401 - import required for xarray but not needed here +from typing import Type +from pytest_lazyfixture import lazy_fixture +from _pytest.python_api import RaisesContext from transport_performance.utils.raster import ( merge_raster_files, sum_resample_file, @@ -159,6 +162,31 @@ def resample_xarr_fpath( return out_filepath +@pytest.fixture +def save_empty_text_file(resample_xarr_fpath: str) -> str: + """Save an empty text file. + + Parameters + ---------- + resample_xarr_fpath : str + File path to dummy raster data, used to make sure file is in the same + directory. + + Returns + ------- + str + Dummy text file name. + + """ + # save an empty text file to the same directory + working_dir = os.path.dirname(resample_xarr_fpath) + test_file_name = "text.txt" + with open(os.path.join(working_dir, test_file_name), "w") as f: + f.write("") + + return test_file_name + + class TestUtilsRaster: """A class to test utils/raster functions.""" @@ -222,9 +250,13 @@ def test_sum_resample_file(self, resample_xarr_fpath: str) -> None: # useful when using -rP flag in pytest to see directory print(f"Temp filepath for resampling test: {resample_xarr_fpath}") - # resample to input and set the output location + # set the output location to sub dir in a different folder + # adding different sub dir to test resolution of issue 121 output_fpath = os.path.join( - os.path.dirname(resample_xarr_fpath), "output.tif" + os.path.dirname(os.path.dirname(resample_xarr_fpath)), + "resample_outputs", + "outputs", + "output.tif", ) sum_resample_file(resample_xarr_fpath, output_fpath) @@ -241,3 +273,49 @@ def test_sum_resample_file(self, resample_xarr_fpath: str) -> None: # assert correct resampling values (summing consitiuent grids) expected_result = np.array([[[14, 22], [46, 54]]]) assert np.array_equal(expected_result, xds_out.to_numpy()) + + @pytest.mark.parametrize( + "input_path, file_name, expected", + [ + # test file that does not exist + ( + lazy_fixture("resample_xarr_fpath"), + "test.tif", + pytest.raises(FileExistsError, match="not found on file."), + ), + # test directory and file that does not exist + ( + lazy_fixture("resample_xarr_fpath"), + os.path.join("test", "test.tif"), + pytest.raises(FileExistsError, match="not found on file."), + ), + # test file with an invalid file extension + ( + lazy_fixture("resample_xarr_fpath"), + lazy_fixture("save_empty_text_file"), + pytest.raises( + ValueError, + match="expected file extension .tif. Found .txt", + ), + ), + ], + ) + def test_sum_resample_on_fail( + self, input_path: str, file_name: str, expected: Type[RaisesContext] + ) -> None: + """Test sum_resample_file in failing cases. + + Parameters + ---------- + input_path : str + path to input dummy raster data + file_name : str + name of file to be tested + expected : Type[RaisesContext] + exception to test with + + """ + with expected: + input_folder = os.path.dirname(input_path) + fpath = os.path.join(input_folder, file_name) + sum_resample_file(fpath, "")