ErikNixdorf / sbat

The Repo for the Surface Water Balance Analysis Tool
BSD 3-Clause "New" or "Revised" License
3 stars 0 forks source link

Plotting should be centralized in a seperate `plot` submodule #55

Closed MarcoHannemann closed 8 months ago

MarcoHannemann commented 1 year ago

Each of the submodules produces results we would like to visualize using matplotlib. Plotting is invoked when plot_results in sbat.yaml is set True.

Currently, each submodule has their own plot functions, e.g. sbat.bflow.plot_along_streamlines. These plot functions are partially again wrapped into a higher plot function such as sbat.bflow.plot_bf_results.

This behavior has some downsides:

1) In sbat.py, the plot functions are decentrally called within the class methods when self.config.plot_results == True 2) The plotting functions pollute the submodules, making them less readable and harder to debug 3) When output == False, plotting fails because there is no output directory defined. However, only plotting when output == True is problematic as sometimes results are not computed because of the tight coupling between plotting and the submodules

At the moment I suggest the following solution, but that can be discussed:

1) We relocate the plotting functions and place them into a separate file, could be for example sbat/plot/plot.py. 2) We create a public class method into the model that is called by the user, that is executed at the end of sbat.main. This method then checks the configuration for present results (as in "Has baseflow been calculated?") and then calls the plot.py. Bonus) We rename sbat.main into sbat.run or sbat.compute, as main is a pretty meaningless name.

When we are at it, we could do the same for writing CSV output to disk.