Repository: mie-lab/trackintel

Issue:
BUG: error in modal split plot when passing axis
The modal split plotting function throws an error if the `axis` argument is not None. The error occurs in [this line](https://github.com/mie-lab/trackintel/blob/ca4300e6056b8500a27907faca515478ca268524/trackintel/visualization/modal_split.py#L112)
because if an axis object is passed as an argument, the `fig` variable does not exist. A simple fix is probably to put that line into an if clause as well.

Discussion:

Patch:
diff --git a/trackintel/visualization/modal_split.py b/trackintel/visualization/modal_split.py
index 83937a1..0448212 100644
--- a/trackintel/visualization/modal_split.py
+++ b/trackintel/visualization/modal_split.py
@@ -8,6 +8,7 @@ def plot_modal_split(
     df_modal_split_in,
     out_path=None,
     date_fmt_x_axis="%W",
+    fig=None,
     axis=None,
     title=None,
     x_label=None,
@@ -33,19 +34,21 @@ def plot_modal_split(
     title : str, optional
     x_label : str, optional
     y_label : str, optional
+    fig : matplotlib.figure
+        Only used if axis is provided as well.
     axis : matplotlib axes
     x_pad : float, default: 10
-        used to set ax.xaxis.labelpad
+        Used to set ax.xaxis.labelpad
     y_pad : float, default: 10
-        used to set ax.yaxis.labelpad
+        Used to set ax.yaxis.labelpad
     title_pad : float, default: 1.02
-        passed on to `matplotlib.pyplot.title`
-    skip_xticks : int, default: 0
-        if larger than 0, every nth x-tick label is skipped.
+        Passed on to `matplotlib.pyplot.title`
+    skip_xticks : int, default: 1
+        Every nth x-tick label is kept.
     n_col_legend : int
-        passed on as `ncol` to matplotlib.pyplot.legend()
+        Passed on as `ncol` to matplotlib.pyplot.legend()
     borderaxespad : float
-        passed on to matplotlib.pyplot.legend()
+        Passed on to matplotlib.pyplot.legend()
 
     Returns
     -------
@@ -86,7 +89,7 @@ def plot_modal_split(
     # skip ticks for X axis
     if skip_xticks > 0:
         for i, tick in enumerate(ax.xaxis.get_major_ticks()):
-            if i % skip_xticks == 0:
+            if i % skip_xticks != 0:
                 tick.set_visible(False)
 
     # We use a nice trick to put the legend out of the plot and to scale it automatically
@@ -105,11 +108,13 @@ def plot_modal_split(
     )
 
     if title is not None:
-        plt.title(title, y=title_pad)
+        ax.set_title(title, y=title_pad)
 
     ax.set_xlabel(x_label)
     ax.set_ylabel(y_label)
-    fig.autofmt_xdate()
+
+    if fig is not None:
+        fig.autofmt_xdate()
 
     plt.tight_layout()
 


Test patch:
diff --git a/tests/visualization/test_modal_split.py b/tests/visualization/test_modal_split.py
index 306a0bd..4df63b7 100644
--- a/tests/visualization/test_modal_split.py
+++ b/tests/visualization/test_modal_split.py
@@ -8,10 +8,11 @@ import pytest
 from trackintel.analysis.modal_split import calculate_modal_split
 from trackintel.io.dataset_reader import read_geolife, geolife_add_modes_to_triplegs
 from trackintel.visualization.modal_split import plot_modal_split
+from trackintel.visualization.util import regular_figure
 
 
 @pytest.fixture
-def get_geolife_triplegs_with_modes():
+def geolife_triplegs_with_modes():
     """Get modal split for a small part of the geolife dataset."""
     pfs, labels = read_geolife(os.path.join("tests", "data", "geolife_modes"))
     pfs, sp = pfs.as_positionfixes.generate_staypoints(method="sliding", dist_threshold=25, time_threshold=5)
@@ -22,7 +23,7 @@ def get_geolife_triplegs_with_modes():
 
 
 @pytest.fixture
-def get_test_triplegs_with_modes():
+def triplegs_with_modes():
     """Get modal split for randomly generated data."""
     n = 200
     day_1_h1 = pd.Timestamp("1970-01-01 00:00:00", tz="utc")
@@ -33,44 +34,39 @@ def get_test_triplegs_with_modes():
     df["user_id"] = np.random.randint(1, 5, size=n)
     df["started_at"] = np.random.randint(1, 30, size=n) * one_day
     df["started_at"] = df["started_at"] + day_1_h1
-
     return df
 
 
 class TestPlot_modal_split:
-    def test_create_plot_geolife(self, get_geolife_triplegs_with_modes):
+    def test_create_plot_geolife(self, geolife_triplegs_with_modes):
         """Check if we can run the plot function with geolife data without error"""
-        modal_split = calculate_modal_split(get_geolife_triplegs_with_modes, freq="d", per_user=False)
+        modal_split = calculate_modal_split(geolife_triplegs_with_modes, freq="d", per_user=False)
         plot_modal_split(modal_split)
-        assert True
 
-    def test_check_dtype_error(self, get_geolife_triplegs_with_modes):
+    def test_check_dtype_error(self, geolife_triplegs_with_modes):
         """Check if error is thrown correctly when index is not datetime
 
         freq=None calculates the modal split over the whole period
         """
-        modal_split = calculate_modal_split(get_geolife_triplegs_with_modes, freq=None, per_user=False)
+        modal_split = calculate_modal_split(geolife_triplegs_with_modes, freq=None, per_user=False)
         with pytest.raises(ValueError):
             plot_modal_split(modal_split)
-        assert True
 
-    def test_multi_user_error(self, get_test_triplegs_with_modes):
+    def test_multi_user_error(self, triplegs_with_modes):
         """Create a modal split plot based on randomly generated test data"""
-        modal_split = calculate_modal_split(get_test_triplegs_with_modes, freq="d", per_user=True, norm=True)
+        modal_split = calculate_modal_split(triplegs_with_modes, freq="d", per_user=True, norm=True)
         with pytest.raises(ValueError):
             plot_modal_split(modal_split)
 
         # make sure that there is no error if the data was correctly created
-        modal_split = calculate_modal_split(get_test_triplegs_with_modes, freq="d", per_user=False, norm=True)
+        modal_split = calculate_modal_split(triplegs_with_modes, freq="d", per_user=False, norm=True)
         plot_modal_split(modal_split)
 
-        assert True
-
-    def test_create_plot_testdata(self, get_test_triplegs_with_modes):
+    def test_create_plot_testdata(self, triplegs_with_modes):
         """Create a modal split plot based on randomly generated test data"""
         tmp_file = os.path.join("tests", "data", "modal_split_plot.png")
 
-        modal_split = calculate_modal_split(get_test_triplegs_with_modes, freq="d", per_user=False, norm=True)
+        modal_split = calculate_modal_split(triplegs_with_modes, freq="d", per_user=False, norm=True)
 
         modal_split = modal_split[["walk", "bike", "train", "car", "bus"]]  # change order for the looks of the plot
         plot_modal_split(
@@ -80,3 +76,28 @@ class TestPlot_modal_split:
         assert os.path.exists(tmp_file)
         os.remove(tmp_file)
         os.remove(tmp_file.replace("png", "pdf"))
+
+    def test_ax_arg(self, triplegs_with_modes):
+        """Test if ax is augmented if passed to function."""
+        _, axis = regular_figure()
+        modal_split = calculate_modal_split(triplegs_with_modes, freq="d", norm=True)
+        xlabel, ylabel, title = "xlabel", "ylabel", "title"
+        dateformat = "%d"
+        _, ax = plot_modal_split(
+            modal_split, date_fmt_x_axis=dateformat, x_label=xlabel, y_label=ylabel, title=title, axis=axis
+        )
+        assert axis is ax
+        assert ax.get_xlabel() == xlabel
+        assert ax.get_ylabel() == ylabel
+        assert ax.get_title() == title
+
+    def test_skip_xticks(self, triplegs_with_modes):
+        """Test if function set right ticks invisible."""
+        modal_split = calculate_modal_split(triplegs_with_modes, freq="d", norm=True)
+        mod = 4  # remove all but the mod 4 ticks
+        _, ax = regular_figure()
+        _, ax = plot_modal_split(modal_split)
+        assert all(t.get_visible() for _, t in enumerate(ax.xaxis.get_major_ticks()))
+        _, ax = regular_figure()
+        _, ax = plot_modal_split(modal_split, skip_xticks=mod)
+        assert all(t.get_visible() == (i % mod == 0) for i, t in enumerate(ax.xaxis.get_major_ticks()))