Make plot kwargs more consistent, closes #180 (#195)

* Alias CL args and update tests Ensure CL tool tests have a good spread of MPL-like and Python-like args e.g. both --x-label and --xlabel. Have updated tests so intensity-map tests have MPL-like args, and powder-map tests have Python-like args. This should give good coverage. * Add missing --pdos in styling doc example * Deprecate Python-style plot kwargs for MPL-style - Also add ymin/ymax test to test_plot.py - Add deprecation tests - Improve formatting on existing deprecations in force_constants.py * Update changelog * Correct ymin, ymax and cmap type hints
pace-neutrons · Oct 26, 2021 · 74b20fa · 74b20fa
1 parent 036f706
commit 74b20fa
Show file tree

Hide file tree

Showing 16 changed files with 209 additions and 131 deletions.
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
@@ -18,6 +18,15 @@
   - Internally, plot theming has been adjusted to rely on Matplotlib
     style contexts. This means user changes and style context are more
     likely to be respected.
+  - Additional aliases for plot arguments in the command-line tools have
+    been added, for example either ``--x-label`` or ``--xlabel`` can be used.
+
+- Changes:
+
+  - ``x_label``, ``y_label``, ``y_min`` and ``y_max`` in ``euphonic.plot``
+    functions have been deprecated in favour of ``xlabel``, ``ylabel``,
+    ``ymin`` and ``ymax`` respectively, to match the Matplotlib arguments
+    they refer to, and to match other arguments like ``vmin``, ``vmax``.
 
 `v0.6.2 <https://github.com/pace-neutrons/Euphonic/compare/v0.6.1...v0.6.2>`_
 ------

diff --git a/doc/source/styling.rst b/doc/source/styling.rst
@@ -34,7 +34,7 @@ Matplotlib style sheet.
 `A number of popular styles are predefined in Matplotlib <https://matplotlib.org/stable/gallery/style_sheets/style_sheets_reference.html>`_
 and may be accessed by name, e.g.::
 
-  euphonic-dos --style seaborn quartz.castep_bin
+  euphonic-dos --pdos --style seaborn quartz.castep_bin
 
 will yield a plot on a grey background with white gridlines.
 

diff --git a/euphonic/cli/dispersion.py b/euphonic/cli/dispersion.py
@@ -10,7 +10,7 @@
 from .utils import (load_data_from_file, get_args, _bands_from_force_constants,
                     _compose_style,
                     _get_q_distance, matplotlib_save_or_show, _get_cli_parser,
-                    _calc_modes_kwargs)
+                    _calc_modes_kwargs, _plot_label_kwargs)
 
 
 def main(params: Optional[List[str]] = None) -> None:
@@ -40,14 +40,8 @@ def main(params: Optional[List[str]] = None) -> None:
 
     spectrum = modes.get_dispersion()
 
-    if args.y_label is None:
-        y_label = f"Energy / {spectrum.y_data.units:~P}"
-    else:
-        y_label = args.y_label
-    if args.x_label is None:
-        x_label = ""
-    else:
-        x_label = args.x_label
+    plot_label_kwargs = _plot_label_kwargs(
+        args, default_ylabel=f"Energy / {spectrum.y_data.units:~P}")
 
     if x_tick_labels:
         spectrum.x_tick_labels = x_tick_labels
@@ -59,10 +53,9 @@ def main(params: Optional[List[str]] = None) -> None:
 
     with matplotlib.style.context(style):
         _ = plot_1d(spectra,
-                    title=args.title,
-                    x_label=x_label,
-                    y_label=y_label,
-                    y_min=args.e_min, y_max=args.e_max)
+                    ymin=args.e_min,
+                    ymax=args.e_max,
+                    **plot_label_kwargs)
         matplotlib_save_or_show(save_filename=args.save_to)
 
 

diff --git a/euphonic/cli/dos.py b/euphonic/cli/dos.py
@@ -11,7 +11,7 @@
                     _calc_modes_kwargs, _compose_style,
                     _get_cli_parser, _get_energy_bins,
                     _grid_spec_from_args, _get_pdos_weighting,
-                    _arrange_pdos_groups)
+                    _arrange_pdos_groups, _plot_label_kwargs)
 
 
 def main(params: Optional[List[str]] = None) -> None:
@@ -67,19 +67,12 @@ def main(params: Optional[List[str]] = None) -> None:
     if args.energy_broadening and not args.adaptive:
         dos = dos.broaden(args.energy_broadening*ebins.units, shape=args.shape)
 
-    if args.x_label is None:
-        x_label = f"Energy / {dos.x_data.units:~P}"
-    else:
-        x_label = args.x_label
-    if args.y_label is None:
-        y_label = ""
-    else:
-        y_label = args.y_label
+    plot_label_kwargs = _plot_label_kwargs(
+        args, default_xlabel=f"Energy / {dos.x_data.units:~P}")
 
     style = _compose_style(user_args=args, base=[base_style])
     with matplotlib.style.context(style):
-        _ = plot_1d(dos, title=args.title, x_label=x_label, y_label=y_label,
-                    y_min=0)
+        _ = plot_1d(dos, ymin=0, **plot_label_kwargs)
         matplotlib_save_or_show(save_filename=args.save_to)
 
 

diff --git a/euphonic/cli/intensity_map.py b/euphonic/cli/intensity_map.py
@@ -10,7 +10,7 @@
 from euphonic.util import get_qpoint_labels
 from euphonic.styles import base_style
 from .utils import (_bands_from_force_constants, _calc_modes_kwargs,
-                    _compose_style,
+                    _compose_style, _plot_label_kwargs,
                     get_args, _get_debye_waller,
                     _get_energy_bins, _get_q_distance,
                     _get_cli_parser, load_data_from_file,
@@ -78,14 +78,8 @@ def main(params: Optional[List[str]] = None) -> None:
             shape=args.shape)
 
     print("Plotting figure")
-    if args.y_label is None:
-        y_label = f"Energy / {spectrum.y_data.units:~P}"
-    else:
-        y_label = args.y_label
-    if args.x_label is None:
-        x_label = ""
-    else:
-        x_label = args.x_label
+    plot_label_kwargs = _plot_label_kwargs(
+        args, default_ylabel=f"Energy / {spectrum.y_data.units:~P}")
 
     if x_tick_labels:
         spectrum.x_tick_labels = x_tick_labels
@@ -98,10 +92,9 @@ def main(params: Optional[List[str]] = None) -> None:
     with matplotlib.style.context(style):
 
         euphonic.plot.plot_2d(spectra,
-                              vmin=args.v_min, vmax=args.v_max,
-                              x_label=x_label,
-                              y_label=y_label,
-                              title=args.title)
+                              vmin=args.vmin,
+                              vmax=args.vmax,
+                              **plot_label_kwargs)
         matplotlib_save_or_show(save_filename=args.save_to)
 
 

diff --git a/euphonic/cli/powder_map.py b/euphonic/cli/powder_map.py
@@ -10,7 +10,7 @@
                                 _get_cli_parser,
                                 _get_debye_waller, _get_energy_bins,
                                 _get_q_distance, _get_pdos_weighting,
-                                _arrange_pdos_groups)
+                                _arrange_pdos_groups, _plot_label_kwargs)
 from euphonic.cli.utils import (force_constants_from_file, get_args,
                                 matplotlib_save_or_show)
 import euphonic.plot
@@ -155,14 +155,9 @@ def main(params: Optional[List[str]] = None) -> None:
             shape=args.shape)
 
     print(f"Plotting figure: max intensity {np.max(spectrum.z_data):~P}")
-    if args.y_label is None:
-        y_label = f"Energy / {spectrum.y_data.units:~P}"
-    else:
-        y_label = args.y_label
-    if args.x_label is None:
-        x_label = f"|q| / {q_min.units:~P}"
-    else:
-        x_label = args.x_label
+    plot_label_kwargs = _plot_label_kwargs(
+        args, default_xlabel=f"|q| / {q_min.units:~P}",
+        default_ylabel=f"Energy / {spectrum.y_data.units:~P}")
 
     if args.disable_widgets:
         base = [base_style]
@@ -171,10 +166,9 @@ def main(params: Optional[List[str]] = None) -> None:
     style = _compose_style(user_args=args, base=base)
     with matplotlib.style.context(style):
         fig = euphonic.plot.plot_2d(spectrum,
-                                    vmin=args.v_min, vmax=args.v_max,
-                                    x_label=x_label,
-                                    y_label=y_label,
-                                    title=args.title)
+                                    vmin=args.vmin,
+                                    vmax=args.vmax,
+                                    **plot_label_kwargs)
 
         if args.disable_widgets is False:
             # TextBox only available from mpl 2.1.0

diff --git a/euphonic/cli/utils.py b/euphonic/cli/utils.py
@@ -412,6 +412,20 @@ def _arrange_pdos_groups(pdos: Spectrum1DCollection,
     return dos
 
 
+def _plot_label_kwargs(args: Namespace, default_xlabel: str = '',
+                       default_ylabel: str = '') -> Dict[str, str]:
+    """Collect title/label arguments that can be passed to plot_nd
+    """
+    plot_kwargs = dict(title=args.title,
+                       xlabel=default_xlabel,
+                       ylabel=default_ylabel)
+    if args.ylabel is not None:
+        plot_kwargs['ylabel'] = args.ylabel
+    if args.xlabel is not None:
+        plot_kwargs['xlabel'] = args.xlabel
+    return plot_kwargs
+
+
 def _calc_modes_kwargs(args: Namespace) -> Dict[str, Any]:
     """Collect arguments that can be passed to calculate_qpoint_phonon_modes()
     """
@@ -525,7 +539,7 @@ def __call__(self, parser, args, values, option_string=None):
             help=('Force use of compiled C extension when computing '
                   'phonon frequencies/eigenvectors (or raise error).'))
         use_c.add_argument(
-            '--disable-c', action='store_false', dest='use_c',
+            '--disable-c', action='store_false', dest='use_c', default=None,
             help=('Do not attempt to use compiled C extension when computing '
                   'phonon frequencies/eigenvectors.'))
         sections['performance'].add_argument(
@@ -636,10 +650,10 @@ def __call__(self, parser, args, values, option_string=None):
             help='Save resulting plot to a file with this name')
         section.add_argument('--title', type=str, default='',
                              help='Plot title')
-        section.add_argument('--x-label', type=str, default=None,
-                             dest='x_label', help='Plot x-axis label')
-        section.add_argument('--y-label', type=str, default=None,
-                             dest='y_label', help='Plot y-axis label')
+        section.add_argument('--x-label', '--xlabel', type=str, default=None,
+                             dest='xlabel', help='Plot x-axis label')
+        section.add_argument('--y-label', '--ylabel', type=str, default=None,
+                             dest='ylabel', help='Plot y-axis label')
         section.add_argument('--style', type=str, nargs='+',
                              help='Matplotlib styles (name or file)')
         section.add_argument('--no-base-style', action='store_true',
@@ -651,17 +665,20 @@ def __call__(self, parser, args, values, option_string=None):
                                    'known to Matplotlib. font-family will be '
                                    'set to sans-serif; it doesn\'t matter if)'
                                    'the font is actually sans-serif.'))
-        section.add_argument('--fontsize', type=float, default=None,
+        section.add_argument('--font-size', '--fontsize', type=float,
+                             default=None, dest='fontsize',
                              help='Set base font size in pt.')
-        section.add_argument('--figsize', type=float, nargs=2, default=None,
+        section.add_argument('--fig-size', '--figsize', type=float, nargs=2,
+                             default=None, dest='figsize',
                              help='Figure canvas size in FIGSIZE-UNITS')
-        section.add_argument('--figsize-unit', type=str, default='cm',
-                             dest='figsize_unit',
+        section.add_argument('--fig-size-unit', '--figsize-unit', type=str,
+                             default='cm', dest='figsize_unit',
                              help='Unit of length for --figsize')
 
     if ('plotting' in features) and not ('map' in features):
         section = sections['plotting']
-        section.add_argument('--linewidth', type=float, default=None,
+        section.add_argument('--line-width', '--linewidth', type=float,
+                             default=None, dest='linewidth',
                              help='Set line width in pt.')
 
     if {'ebins', 'q-e'}.intersection(features):
@@ -726,13 +743,14 @@ def __call__(self, parser, args, values, option_string=None):
 
     if 'map' in features:
         sections['plotting'].add_argument(
-            '--v-min', type=float, default=None, dest='v_min',
+            '--v-min', '--vmin', type=float, default=None, dest='vmin',
             help='Minimum of data range for colormap.')
         sections['plotting'].add_argument(
-            '--v-max', type=float, default=None, dest='v_max',
+            '--v-max', '--vmax', type=float, default=None, dest='vmax',
             help='Maximum of data range for colormap.')
         sections['plotting'].add_argument(
-            '--cmap', type=str, default=None, help='Matplotlib colormap')
+            '--c-map', '--cmap', type=str, default=None, dest='cmap',
+            help='Matplotlib colormap')
 
     if 'btol' in features:
         sections['q'].add_argument(

diff --git a/euphonic/force_constants.py b/euphonic/force_constants.py
@@ -18,7 +18,8 @@
 from euphonic.readers import castep, phonopy
 from euphonic.util import (is_gamma, get_all_origins,
                            mode_gradients_to_widths,
-                           _get_supercell_relative_idx)
+                           _get_supercell_relative_idx,
+                           _deprecation_warn)
 from euphonic import (ureg, Quantity, Crystal, QpointPhononModes,
                       QpointFrequencies)
 
@@ -203,9 +204,8 @@ def calculate_qpoint_phonon_modes(
             euphonic-optimise-dipole-parameter program for help on choosing
             a good dipole_parameter.
         eta_scale
-
             .. deprecated:: 0.6.0
-            Please use dipole_parameter instead
+               Please use dipole_parameter instead
         splitting
             Whether to calculate the LO-TO splitting at the gamma
             points. Only applied if dipole is True and the Born charges
@@ -242,15 +242,14 @@ def calculate_qpoint_phonon_modes(
             widths and used in adaptive broadening for DOS. For details
             on how these are calculated see the Notes section
         return_mode_widths
-
             .. deprecated:: 0.5.2
-            The mode widths as calculated were only applicable for
-            adaptive broadening of DOS, this argument will be removed
-            in favour of the more flexible return_mode_gradients,
-            which will allow the calculation of direction-specific
-            mode widths in the future, for example. The mode widths
-            can still be obtained from the mode gradients using
-            euphonic.util.mode_gradients_to_widths
+               The mode widths as calculated were only applicable for
+               adaptive broadening of DOS, this argument will be removed
+               in favour of the more flexible return_mode_gradients,
+               which will allow the calculation of direction-specific
+               mode widths in the future, for example. The mode widths
+               can still be obtained from the mode gradients using
+               euphonic.util.mode_gradients_to_widths
 
         Returns
         -------
@@ -483,10 +482,7 @@ def _calculate_phonons_at_qpts(
                 category=DeprecationWarning, stacklevel=3)
             return_mode_gradients = True
         if eta_scale != 1.0:
-            warnings.warn(
-                'eta_scale has been deprecated and will be removed '
-                'in a future release. Please use dipole_parameter instead ',
-                category=DeprecationWarning, stacklevel=3)
+            _deprecation_warn('eta_scale', 'dipole_parameter', stacklevel=4)
             dipole_parameter = eta_scale
         # Check weights is of appropriate type and shape, to avoid doing all
         # the interpolation only for it to fail creating QpointPhononModes