Repository: sky-uk/anticipy

Issue:
Make path an optional input variable in forecast_plot.plot_forecast()
When using `output='jupyter'`, no path is needed.
When `output='png'` or `output='html'` we need to verify path is not empty.

Discussion:
slenas: @capelastegui @bezes guys could you please check my reply above and comment? As I'm seeing it, for me "path" is not exactly an optional parameter...
codecov-io: # [Codecov](https://codecov.io/gh/sky-uk/anticipy/pull/71?src=pr&el=h1) Report
> Merging [#71](https://codecov.io/gh/sky-uk/anticipy/pull/71?src=pr&el=desc) into [master](https://codecov.io/gh/sky-uk/anticipy/commit/abf46202f20ef148073903593d2ec571ca8b0793?src=pr&el=desc) will **decrease** coverage by `0.24%`.
> The diff coverage is `37.5%`.

[![Impacted file tree graph](https://codecov.io/gh/sky-uk/anticipy/pull/71/graphs/tree.svg?width=650&token=QEpdZ0YcB4&height=150&src=pr)](https://codecov.io/gh/sky-uk/anticipy/pull/71?src=pr&el=tree)

```diff
@@            Coverage Diff            @@
##           master     #71      +/-   ##
=========================================
- Coverage   84.85%   84.6%   -0.25%     
=========================================
  Files           6       6              
  Lines        1373    1377       +4     
=========================================
  Hits         1165    1165              
- Misses        208     212       +4
```


| [Impacted Files](https://codecov.io/gh/sky-uk/anticipy/pull/71?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [anticipy/app.py](https://codecov.io/gh/sky-uk/anticipy/pull/71/diff?src=pr&el=tree#diff-YW50aWNpcHkvYXBwLnB5) | `0% <0%> (ø)` | :arrow_up: |
| [anticipy/forecast\_plot.py](https://codecov.io/gh/sky-uk/anticipy/pull/71/diff?src=pr&el=tree#diff-YW50aWNpcHkvZm9yZWNhc3RfcGxvdC5weQ==) | `85.33% <42.85%> (-2.17%)` | :arrow_down: |
| [anticipy/forecast\_models.py](https://codecov.io/gh/sky-uk/anticipy/pull/71/diff?src=pr&el=tree#diff-YW50aWNpcHkvZm9yZWNhc3RfbW9kZWxzLnB5) | `86.84% <0%> (-0.05%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/sky-uk/anticipy/pull/71?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/sky-uk/anticipy/pull/71?src=pr&el=footer). Last update [abf4620...2782ecd](https://codecov.io/gh/sky-uk/anticipy/pull/71?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/anticipy/app.py b/anticipy/app.py
index 91ab7ff..05511cc 100644
--- a/anticipy/app.py
+++ b/anticipy/app.py
@@ -95,9 +95,8 @@ def run_forecast_app(path_in, path_out=None, forecast_years=2.0,
     df_metadata.to_csv(path_metadata, index=False)
 
     try:
-        forecast_plot.plot_forecast(
-            df_result, path_plot, output=output_format, width=1920,
-            height=1080)
+        forecast_plot.plot_forecast(df_result, output_format, path_plot,
+                                    width=1920, height=1080)
     except AssertionError:
         logger.info("Couldn't generate plot - The required plotting library "
                     "is not installed")
diff --git a/anticipy/forecast_plot.py b/anticipy/forecast_plot.py
index 7427ccc..889a667 100644
--- a/anticipy/forecast_plot.py
+++ b/anticipy/forecast_plot.py
@@ -322,9 +322,8 @@ def _plotly_forecast_create(df_fcast, subplots, sources, nrows, ncols,
     return fig
 
 
-def plot_forecast(df_fcast, path, output='html', width=None,
-                  height=None, title=None, dpi=70, show_legend=True,
-                  auto_open=False):
+def plot_forecast(df_fcast, output, path=None, width=None, height=None,
+                  title=None, dpi=70, show_legend=True, auto_open=False):
     """
     Generates matplotlib or plotly plot and saves it respectively as png or
     html
@@ -334,12 +333,13 @@ def plot_forecast(df_fcast, path, output='html', width=None,
         | - date (timestamp)
         | - model (str) : ID for the forecast model
         | - y (float) : Value of the time series in that sample
-        | - is_actuals (bool) : True for actuals samples, False for forecasted samples # noqa
+        | - is_actuals (bool) : True for actuals samples, False for
+        |                       forecasted samples # noqa
     :type df_fcast: pandas.DataFrame
+    :param output: Indicates the output type (html=Default, png or jupyter)
+    :type output: basestring
     :param path: File path for output
     :type path: basestring
-    :param output: ...
-    :type output: ...
     :param width: Image width, in pixels
     :type width: int
     :param height: Image height, in pixels
@@ -353,10 +353,17 @@ def plot_forecast(df_fcast, path, output='html', width=None,
     :param auto_open: Indicates whether the output will be displayed
                       automatically
     :type auto_open: bool
+
+    :return: Success or failure code.
+    :rtype: int
     """
 
     assert isinstance(df_fcast, pd.DataFrame)
 
+    if not path and (output == 'html' or output == 'png'):
+        logger.error('No export path provided.')
+        return 1
+
     if 'source' in df_fcast.columns:
         subplots = True
         sources = df_fcast.loc[df_fcast['is_actuals'], 'source'].unique()
@@ -375,6 +382,7 @@ def plot_forecast(df_fcast, path, output='html', width=None,
             fig = _matplotlib_forecast_create(df_fcast, subplots, sources,
                                               nrows, ncols, width, height,
                                               title, dpi, show_legend)
+
             path = '{}.png'.format(path)
             dirname, fname = os.path.split(path)
             if not os.path.exists(dirname):
@@ -412,5 +420,6 @@ def plot_forecast(df_fcast, path, output='html', width=None,
     else:
         logger.error('Wrong exporting format provided. Either png, html or '
                      'jupyter formats are supported at the moment.')
-        return 0
+        return 1
 
+    return 0
diff --git a/docs/source/tutorial.rst b/docs/source/tutorial.rst
index df92fa1..5278592 100755
--- a/docs/source/tutorial.rst
+++ b/docs/source/tutorial.rst
@@ -41,19 +41,20 @@ The tool automatically selects the best model from a list of defaults - in this
 users can instead provide their preferred model or lists of candidate models, as explained in :ref:`models`.
 
 You can plot the forecast output using the functions in :py:mod:`forecast_plot`.
-:py:func:`anticipy.forecast_plot.plot_forecast_save` saves the plot as a file. If you use
-nsa.forecast in a Jupyter notebook, :py:func:`anticipy.forecast_plot.plot_forecast` shows the plot as the output of
-a notebook cell. The plot looks as follows:
+:py:func:`anticipy.forecast_plot.plot_forecast` saves the plot as a file or
+exports it to a jupyter notebook. The plot looks as follows:
 
 .. image:: resources/tutorial-forecast1.png
 
 The code to generate the plot is::
 
-    path_tutorial_plot = 'plot-tutorial.png'
+    path_tutorial_plot = 'plot-tutorial'
     # Save plot as a file
-    forecast_plot.plot_forecast_save(df_forecast, path_tutorial_plot,width=350, height=240, title='Tutorial Forecast')
+    forecast_plot.plot_forecast(df_forecast, output='html',
+    path=path_tutorial_plot, width=350, height=240, title='Tutorial Forecast')
     # Show plot in a jupyter notebook
-    forecast_plot.plot_forecast(df_forecast, width=350, height=240, title='Tutorial Forecast')
+    forecast_plot.plot_forecast(df_forecast, output='jupyter', width=350,
+    height=240, title='Tutorial Forecast')
 
 
 


Test patch:
diff --git a/tests/test_forecast_plot.py b/tests/test_forecast_plot.py
index cc97abb..c50a2cf 100644
--- a/tests/test_forecast_plot.py
+++ b/tests/test_forecast_plot.py
@@ -96,59 +96,85 @@ class TestForecastPlot(PandasTest):
         if not forecast_plot._matplotlib_imported:
             self.skipTest('Test skipped as Matplotlib is not installed...')
         path = get_file_path(base_folder, 'test_mpl')
-        forecast_plot.plot_forecast(df_forecast, path, 'png', 900, 600,
+        forecast_plot.plot_forecast(df_forecast, 'png', path, 900, 600,
                                     'Test Plot', show_legend=False,
                                     auto_open=False)
         self.assertTrue(os.path.isfile('{}.png'.format(path)))
 
         path = get_file_path(base_folder, 'test_facet_mpl')
-        forecast_plot.plot_forecast(df_forecast_facet, path, 'png', 1200, 900,
+        forecast_plot.plot_forecast(df_forecast_facet, 'png', path, 1200, 900,
                                     'Test Plot', show_legend=True,
                                     auto_open=False)
         self.assertTrue(os.path.isfile('{}.png'.format(path)))
 
         # Repeat test with prediction intervals
         path = get_file_path(base_folder, 'test_pi_mpl')
-        forecast_plot.plot_forecast(df_forecast_pi, path, 'png', 900, 600,
+        forecast_plot.plot_forecast(df_forecast_pi, 'png', path, 900, 600,
                                     'Test Plot', show_legend=True,
                                     auto_open=False)
         self.assertTrue(os.path.isfile('{}.png'.format(path)))
 
         path = get_file_path(base_folder, 'test_pi_facet_mpl')
-        forecast_plot.plot_forecast(df_forecast_pi_facet, path, 'png', 1200,
+        forecast_plot.plot_forecast(df_forecast_pi_facet, 'png', path, 1200,
                                     900, 'Test Plot', show_legend=True,
                                     auto_open=False)
         self.assertTrue(os.path.isfile('{}.png'.format(path)))
 
+        # Test the case where a 'None' or an empty path is provided
+        self.assertTrue(forecast_plot.plot_forecast(df_forecast_pi_facet,
+                                                    'png', None, 1200, 900,
+                                                    'Test Plot',
+                                                    show_legend=True,
+                                                    auto_open=False))
+
+        self.assertTrue(forecast_plot.plot_forecast(df_forecast_pi_facet,
+                                                    'png', '', 1200, 900,
+                                                    'Test Plot',
+                                                    show_legend=True,
+                                                    auto_open=False))
+
     def test_plot_forecast_html(self):
         if not forecast_plot._plotly_imported:
             self.skipTest('Test skipped as Plotly is not installed...')
 
         path = get_file_path(base_folder, 'test_plotly')
-        forecast_plot.plot_forecast(df_forecast, path, 'html', 900, 600,
+        forecast_plot.plot_forecast(df_forecast, 'html', path, 900, 600,
                                     'Test Plot', show_legend=False,
                                     auto_open=False)
         self.assertTrue(os.path.isfile('{}.html'.format(path)))
 
         path = get_file_path(base_folder, 'test_facet_plotly')
-        forecast_plot.plot_forecast(df_forecast_facet, path, 'html', 1900,
+        forecast_plot.plot_forecast(df_forecast_facet, 'html', path, 1900,
                                     1200, 'Test Plot', show_legend=False,
                                     auto_open=False)
         self.assertTrue(os.path.isfile('{}.html'.format(path)))
 
         # Repeat test with prediction intervals
         path = get_file_path(base_folder, 'test_pi_plotly')
-        forecast_plot.plot_forecast(df_forecast_pi, path, 'html', 900, 600,
+        forecast_plot.plot_forecast(df_forecast_pi, 'html', path, 900, 600,
                                     'Test Plot', show_legend=False,
                                     auto_open=False)
         self.assertTrue(os.path.isfile('{}.html'.format(path)))
 
         path = get_file_path(base_folder, 'test_pi_facet_plotly')
-        forecast_plot.plot_forecast(df_forecast_pi_facet, path, 'html', 1900,
+        forecast_plot.plot_forecast(df_forecast_pi_facet, 'html', path, 1900,
                                     1200, 'Test Plot', show_legend=False,
                                     auto_open=False)
         self.assertTrue(os.path.isfile('{}.html'.format(path)))
 
+        # Test the case where a 'None' or an empty path is provided
+        self.assertTrue(forecast_plot.plot_forecast(df_forecast_pi_facet,
+                                                    'html', None, 1900, 1200,
+                                                    'Test Plot',
+                                                    show_legend=False,
+                                                    auto_open=False))
+
+        self.assertTrue(forecast_plot.plot_forecast(df_forecast_pi_facet,
+                                                    'html', '', 1900, 1200,
+                                                    'Test Plot',
+                                                    show_legend=False,
+                                                    auto_open=False))
+
     def test_plot_forecast_jupyter(self):
         if (not forecast_plot._plotly_imported) or \
                 (not forecast_plot._ipython_imported):
@@ -156,7 +182,6 @@ class TestForecastPlot(PandasTest):
                           'not installed...')
 
         forecast_plot.plot_forecast(df_forecast_pi_facet,
-                                    path=None,
                                     output='jupyter',
                                     width=1900,
                                     height=1200,