Repository: CORE-GATECH-GROUP/serpent-tools (language: python)

Issue:
ENH Allow underlying results plot to be configured with keyword arguments
**Is your feature request related to a problem? Please describe.**
The ResultsReader has a very nice plot method, but I've found it to be frustrating to configure and tweak at times. Currently, there is no way to manually add markers, colors, tweak the draw style (line, steps, dashed, etc)

**Describe the solution you'd like**
By allowing the user to pass arguments as keyword arguments through to `ax.plot` and `ax.errorbar`, we can easily allow this configuration

https://github.com/CORE-GATECH-GROUP/serpent-tools/blob/ee88bf4c2e17901aba2101daba53d384313e48d2/serpentTools/parsers/results.py#L852-L857

The patch should be merged into the main branch rather than develop since there's a lot going on currently with respect to 0.10.0 and #335, #400 

**Describe alternatives you've considered**
The workaround is to extract and plot the data on your own, but we can do a little better

This is related to #417, but a smaller, more focused issue

DEP Deprecate steps plot argument in favor of matplotlib drawstyle
Initially brought up in #432 for the `HomogUniv` class. As we start forwarding more plot options to the underlying matplotlib plot functions (#417, #423, #422 and friends) there may be a point where we want to rely on users passing `drawstyle=<option>` rather than our own logic for the `step` argument.

Some examples

 https://github.com/CORE-GATECH-GROUP/serpent-tools/blob/67dc469e2697dfffbd6c06fa1bc20eea306383c9/serpentTools/objects/containers.py#L411 https://github.com/CORE-GATECH-GROUP/serpent-tools/blob/67dc469e2697dfffbd6c06fa1bc20eea306383c9/serpentTools/detectors.py#L561-L566 https://github.com/CORE-GATECH-GROUP/serpent-tools/blob/67dc469e2697dfffbd6c06fa1bc20eea306383c9/serpentTools/objects/xsdata.py#L363 https://github.com/CORE-GATECH-GROUP/serpent-tools/blob/67dc469e2697dfffbd6c06fa1bc20eea306383c9/serpentTools/parsers/sensitivity.py#L408-L409

## Roadmap

I propose that by 0.10.0 we support passing `steps` and `drawstyle` for the methods that currently support `steps`. Methods like the sensitivity plotter can have a default `drawstyle` using `kwargs.setdefault("drawstyle", ...)`. 

Instances where both are passed should raise a DeprecationWarning by 0.10.0. Following 0.11.0 we should no longer support the `steps` argument, instead falling back to the matplotlib `drawstyle` argument.

Instances where we currently allow `steps` and additional `kwargs` should raise a pending deprecation warning by 0.9.4

## Extras

Some care should be taken for the spectrum plot and other cases where we perform a line plot with some uncertainties. Here we wish to draw the errorbars in the middle of the bin (#428)
ENH Forward more arguments to MPL plot routines
A handful of our plot routines don't allow the user to pass on additional arguments to the underlying plot call, making configuration and tweaking a little frustrating. I'll highlight some here in no particular order that I think we should allow. Basically it would boil down to allowing `**kwargs` in the method signature, and then sending those directly to `ax.plot` or `ax.errorbar`, depending on how the plot is drawn.

- [x] sensitivity
https://github.com/CORE-GATECH-GROUP/serpent-tools/blob/ee88bf4c2e17901aba2101daba53d384313e48d2/serpentTools/parsers/sensitivity.py#L408-L409
- [ ] results
https://github.com/CORE-GATECH-GROUP/serpent-tools/blob/ee88bf4c2e17901aba2101daba53d384313e48d2/serpentTools/parsers/results.py#L852-L857
The last line here should be `ax.plot` rather than `ax.errorbar` too
- [x] homog univ
https://github.com/CORE-GATECH-GROUP/serpent-tools/blob/ee88bf4c2e17901aba2101daba53d384313e48d2/serpentTools/objects/containers.py#L454-L455


Discussion / hints:

Fix patch:
diff --git a/serpentTools/parsers/results.py b/serpentTools/parsers/results.py
index f0444a07..64bb6a61 100644
--- a/serpentTools/parsers/results.py
+++ b/serpentTools/parsers/results.py
@@ -715,7 +715,7 @@ def _cleanMetadata(self):
     @magicPlotDocDecorator
     def plot(self, x, y=None, right=None, sigma=3, ax=None, legend=None,
              ncol=None, xlabel=True, ylabel=None, logx=False, logy=False,
-             loglog=False, rightlabel=None):
+             loglog=False, rightlabel=None, **kwargs):
         """
         Plot quantities over time
 
@@ -756,6 +756,11 @@ def plot(self, x, y=None, right=None, sigma=3, ax=None, legend=None,
         rlabel: str or None
             If given and passing values to ``right``, use this to label
             the y-axis.
+        {kwargs} : pyplot plot or errorbar depending on if ``sigma`` evaluates
+            to ``True``. Will be passed for both right and left plots. Passing
+            ``"label"`` to label a single plot is not allowed. Instead, pass
+            a dictionary for ``y`` and/or ``right``. Values will be used to
+            label a given quantity.
 
         Returns
         -------
@@ -781,15 +786,17 @@ def plot(self, x, y=None, right=None, sigma=3, ax=None, legend=None,
         if xlabel is True:
             xlabel = RESULTS_PLOT_XLABELS[x]
 
+        if "label" in kwargs:
+            raise KeyError("Passing label as kwargs is not supported")
+
         if len(y) == 1 and ylabel is None:
-            for ylabel in y.values():
-                break  # just need the first one
+            ylabel = next(iter(y.values()))
             if sigma:
                 ylabel += r'$ \pm {}\sigma$'.format(sigma)
 
         ax = ax or gca()
 
-        self._plot(x, y, ax, sigma)
+        self._plot(x, y, ax, sigma, **kwargs)
 
         if right is None:
             formatPlot(ax, logx=logx, logy=logy, loglog=loglog,
@@ -805,7 +812,7 @@ def plot(self, x, y=None, right=None, sigma=3, ax=None, legend=None,
         colors = colors[len(y):] + colors[:len(y)]
         other.set_prop_cycle(cycler('color', colors))
 
-        self._plot(x, right, other, sigma)
+        self._plot(x, right, other, sigma, **kwargs)
 
         # formatting
         if logy is None or isinstance(logy, bool):
@@ -824,8 +831,7 @@ def plot(self, x, y=None, right=None, sigma=3, ax=None, legend=None,
                 ncol=ncol)
 
         if len(right) == 1 and rightlabel is None:
-            for rightlabel in right.values():
-                break  # just need the first one
+            rightlabel = next(iter(right.values()))
             if sigma:
                 rightlabel += r'$ \pm {}\sigma$'.format(sigma)
 
@@ -836,7 +842,7 @@ def plot(self, x, y=None, right=None, sigma=3, ax=None, legend=None,
 
         return ax, other
 
-    def _plot(self, x, y, ax, sigma):
+    def _plot(self, x, y, ax, sigma, **kwargs):
         """Simple, unformatted plot with dictionary of keys"""
         # get plot data
         xvals = self.resdata[x][:, 0]
@@ -849,12 +855,15 @@ def _plot(self, x, y, ax, sigma):
 
             # grab second column for uncertainty
             if sigma and ydata.shape[1] > 1:
-                ax.errorbar(xvals, ydata[:, 0], label=label,
-                            yerr=ydata[:, 0] * sigma * ydata[:, 1],
-                            )
+                ax.errorbar(
+                    xvals,
+                    ydata[:, 0],
+                    label=label,
+                    yerr=ydata[:, 0] * sigma * ydata[:, 1],
+                    **kwargs,
+                )
             else:
-                ax.errorbar(xvals, ydata[:, 0], label=label,
-                            )
+                ax.plot(xvals, ydata[:, 0], label=label, **kwargs)
 
     @staticmethod
     def _expandPlotIterables(y, tail=''):


Test patch:
diff --git a/tests/plots/result_images/testResultMulti.png b/tests/plots/result_images/testResultMulti.png
new file mode 100644
index 00000000..dc2dd7f2
Binary files /dev/null and b/tests/plots/result_images/testResultMulti.png differ
diff --git a/tests/plots/result_images/testResultWithRight.png b/tests/plots/result_images/testResultWithRight.png
new file mode 100644
index 00000000..53ea851a
Binary files /dev/null and b/tests/plots/result_images/testResultWithRight.png differ
diff --git a/tests/plots/test_results.py b/tests/plots/test_results.py
new file mode 100644
index 00000000..27a179f6
--- /dev/null
+++ b/tests/plots/test_results.py
@@ -0,0 +1,33 @@
+import pytest
+from serpentTools.settings import rc
+from serpentTools.data import readDataFile
+
+from . import compare_or_update_plot
+
+
+@pytest.fixture
+def innerAssem():
+    with rc:
+        rc["serpentVersion"] = "2.1.30"
+        reader = readDataFile("InnerAssembly_res.m")
+    return reader
+
+
+@compare_or_update_plot
+def testResultWithRight(innerAssem):
+    innerAssem.plot(
+        {"absKeff": "keff"},
+        right="conversionRatio",
+        drawstyle="steps-post",
+    )
+
+
+@compare_or_update_plot
+def testResultMulti(innerAssem):
+    innerAssem.plot(
+        "burnup",
+        ["avgVirtCol", "avgRealCol"],
+        ylabel="Collision information",
+        sigma=0,
+        marker=">",
+    )