diff --git a/.github/workflows/check_changelog.yml b/.github/workflows/check_changelog.yml new file mode 100644 index 000000000..b2089be78 --- /dev/null +++ b/.github/workflows/check_changelog.yml @@ -0,0 +1,23 @@ +name: Changelog updated + +on: + pull_request: + types: [opened, reopened, labeled, synchronize] + +jobs: + check_log: + runs-on: ubuntu-latest + steps: + - name: Checkout code + uses: actions/checkout@v6 + with: + fetch-depth: 0 + - name: Get changes + run: | + set +e # Don't immediately fail when bash sees exit 1 + git diff --exit-code "origin/${GITHUB_BASE_REF}" -- CHANGELOG.rst + if [ $? -eq 1 ]; then + exit 0 + fi + # git-diff did not return 1; either no changes or another error encountered + exit 1 diff --git a/.github/workflows/run_tests.yml b/.github/workflows/run_tests.yml index f210d019b..41e603b4f 100644 --- a/.github/workflows/run_tests.yml +++ b/.github/workflows/run_tests.yml @@ -8,6 +8,12 @@ on: pull_request: types: [opened, reopened, labeled, synchronize] workflow_dispatch: + inputs: + push_coverage_badge: + description: 'Push coverage badge (even if not running on master)' + required: true + type: boolean + default: false jobs: lint: @@ -97,4 +103,42 @@ jobs: - name: Publish test results uses: EnricoMi/publish-unit-test-result-action@v2 with: - junit_files: artifacts/**/junit_report*.xml + files: artifacts/**/junit_report*.xml + + publish-coverage: + needs: test + runs-on: ubuntu-latest + steps: + - name: Download Artifacts + uses: actions/download-artifact@v4 + with: + path: artifacts + name: 'Coverage XML (ubuntu-latest)' + + - name: Get Coverage + uses: orgoro/coverage@v3.2 + continue-on-error: true + with: + coverageFile: 'artifacts/coverage-combined.xml' + token: ${{ secrets.GITHUB_TOKEN }} + + publish-coverage-badge: + needs: test + runs-on: ubuntu-latest + if: ${{(github.event_name == 'push' && github.ref == 'refs/heads/master') || (github.event_name == 'workflow_dispatch' && inputs.push_coverage_badge)}} + steps: + - uses: actions/checkout@v6 + with: + ref: badges + fetch-depth: 0 + + - name: Download Artifacts + uses: actions/download-artifact@v4 + with: + path: . + name: 'Coverage Badge (ubuntu-latest)' + + - uses: EndBug/add-and-commit@v9 + with: + add: 'badge_shieldsio_linecoverage_lightgrey.svg' + message: 'Update coverage badge' diff --git a/.github/workflows/test_checkout_one_os.yml b/.github/workflows/test_checkout_one_os.yml index 3c694fb78..b48b9b793 100644 --- a/.github/workflows/test_checkout_one_os.yml +++ b/.github/workflows/test_checkout_one_os.yml @@ -95,16 +95,39 @@ jobs: name: Unit test results ${{ inputs.os }} path: tests_and_analysis/test/reports/junit_report*.xml - - name: Publish Codacy coverage - uses: codacy/codacy-coverage-reporter-action@v1 + - name: Merge coverage reports if: inputs.coverage + shell: bash -l {0} + run: | + dotnet tool install -g dotnet-coverage + dotnet coverage merge --reports "tests_and_analysis/test/reports/coverage*.xml" -f cobertura -o "tests_and_analysis/test/reports/coverage-combined.xml" + + - name: Create coverage outputs + if: inputs.coverage + shell: bash -l {0} + run: | + dotnet tool install --global dotnet-reportgenerator-globaltool --version 5.5.1 + reportgenerator -reports:tests_and_analysis/test/reports/coverage-combined.xml -targetdir:tests_and_analysis/test/reports/html -reporttypes:Html_Dark + reportgenerator -reports:tests_and_analysis/test/reports/coverage-combined.xml -targetdir:tests_and_analysis/test/reports -reporttypes:Badges + + - name: Upload coverage XML + if: inputs.coverage + uses: actions/upload-artifact@v4 with: - project-token: ${{ secrets.codacy_project_token }} - coverage-reports: tests_and_analysis/test/reports/coverage*.xml + name: Coverage XML (${{ inputs.os }}) + path: tests_and_analysis/test/reports/coverage-combined.xml - - uses: codecov/codecov-action@v4 + - name: Upload coverage HTML if: inputs.coverage + uses: actions/upload-artifact@v4 + with: + name: Coverage HTML (${{ inputs.os }}) + path: tests_and_analysis/test/reports/html + include-hidden-files: true + + - name: Upload coverage Badge + if: inputs.coverage + uses: actions/upload-artifact@v4 with: - files: tests_and_analysis/test/reports/coverage*.xml - token: ${{ secrets.CODECOV_TOKEN }} - fail_ci_if_error: true + name: Coverage Badge (${{ inputs.os }}) + path: tests_and_analysis/test/reports/badge_shieldsio_linecoverage_lightgrey.svg diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 1b4fb8097..7e3f6a4fb 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -1,8 +1,47 @@ `Unreleased `_ ------------------------------------------------------------------------------- +- API changes + + - Public functions in ``euphonic.powder`` now use mandatory keyword arguments + - This will break code that depends on these arguments being in a specific order + + - The various Spectrum classes have an ``assert_regular_bins`` + method. It is now forbidden to use positional arguments and in the + 2D cases ``bin_ax`` has a default value of "y". This makes the API + safer and more formally correct. + + - Unused argument ``use_brille`` is removed from + ``euphonic.cli.brille_convergence.check_brille_settings``. + + - "Adaptive fit" parameter is removed from spectrum broaden() + methods and euphonic-dos; "cubic" parametrisation is removed and + superior "cheby-log" fit always used. + +- Features + + - Spectrum1DCollection and Spectrum2DCollection can be indexed with + slices where the stop value exceeds the collection + length. (e.g. if a collection of 5 spectra is indexed with [3:10] + it will return a collection with the spectra at indices 3 and 4.) + This is consistent with the behaviour of Python lists and numpy + arrays. + + Previously this would raise an IndexError. Technically it is a + **breaking change** as somebody's code could depend on this + IndexError. At this stage it seems an acceptable risk. + +- Other changes + + - Error messages have been overhauled and now follow a consistent format:: + + summary + + [reason] + + fix + `v1.6.0 `_ ------------------------------------------------------------------------------ - Requirements @@ -14,7 +53,7 @@ - Security - Bumped *wheel* requirement for docs and testing to 0.46.2. (`CVE-2026-24049 `_) - + - Bug fixes diff --git a/README.rst b/README.rst index e767d806f..8dbdca78a 100644 --- a/README.rst +++ b/README.rst @@ -2,7 +2,7 @@ Euphonic ======== -|PyPI Version| |Conda Version| |Documentation Status| |Tests| |License| |DOI| +|PyPI Version| |Conda Version| |Documentation Status| |Tests| |Coverage| |License| |DOI| .. |PyPI Version| image:: https://img.shields.io/pypi/v/euphonic :target: https://pypi.org/project/euphonic/ @@ -20,6 +20,9 @@ Euphonic :target: https://github.com/pace-neutrons/Euphonic/actions/workflows/run_tests.yml :alt: Tests +.. |Coverage| image:: https://raw.githubusercontent.com/pace-neutrons/Euphonic/refs/heads/badges/badge_shieldsio_linecoverage_lightgrey.svg + :target: https://github.com/pace-neutrons/Euphonic/actions/workflows/run_tests.yml :alt: Coverage + .. |License| image:: https://img.shields.io/pypi/l/euphonic :target: https://github.com/pace-neutrons/Euphonic/blob/master/LICENSE :alt: License diff --git a/euphonic/brille.py b/euphonic/brille.py index 6a1db0494..43c93ba6f 100644 --- a/euphonic/brille.py +++ b/euphonic/brille.py @@ -10,9 +10,11 @@ import brille as br except ModuleNotFoundError as err: err_msg = textwrap.dedent(""" - Cannot import Brille for use with BrilleInterpolator - (maybe Brille is not installed?). To install Euphonic's - optional Brille dependency, try: + Cannot import Brille for use with BrilleInterpolator. + + This may be because Brille is not installed. + + To install Euphonic's optional Brille dependency, try: pip install euphonic[brille] """) @@ -25,6 +27,7 @@ QpointPhononModes, ureg, ) +from euphonic.util import comma_join, format_error from euphonic.validate import _check_constructor_inputs @@ -194,7 +197,11 @@ def from_force_constants( """ grid_type_opts = ('trellis', 'mesh', 'nest') if grid_type not in grid_type_opts: - msg = f'Grid type "{grid_type}" not recognised' + msg = format_error( + f'Grid type "{grid_type}" not recognised.', + fix=('Acceptable grid types are:' + f' {comma_join(grid_type_opts)}.'), + ) raise ValueError(msg) crystal = force_constants.crystal diff --git a/euphonic/broadening.py b/euphonic/broadening.py index 69b026398..485d52d83 100644 --- a/euphonic/broadening.py +++ b/euphonic/broadening.py @@ -13,9 +13,8 @@ from scipy.stats import norm from euphonic.ureg import ureg -from euphonic.util import dedent_and_fill +from euphonic.util import format_error -ErrorFit = Literal['cheby-log', 'cubic'] KernelShape = Literal['gauss', 'lorentz'] @@ -33,7 +32,6 @@ def variable_width_broadening( width_convention: Literal['fwhm', 'std'] = 'fwhm', adaptive_error: float = 1e-2, shape: KernelShape = 'gauss', - fit: ErrorFit = 'cheby-log', ) -> Quantity: r"""Apply x-dependent Gaussian broadening to 1-D data series @@ -70,25 +68,24 @@ def variable_width_broadening( approximate gaussians. shape Select broadening kernel function. - fit - Select parametrisation of kernel width spacing to adaptive_error. - 'cheby-log' is recommended: for shape 'gauss', 'cubic' is also - available. """ if width_convention.lower() == 'fwhm' and shape == 'gauss': def sigma_function(x: Quantity) -> Quantity: return width_function(x) * FWHM_TO_SIGMA elif width_convention.lower() == 'std' and shape == 'lorentz': - msg = ( - 'Standard deviation unavailable for Lorentzian ' - 'function: please use FWHM.' + msg = format_error( + 'Standard deviation unavailable.', + fix='For Lorentzian function: please use FWHM.', ) raise ValueError(msg) elif width_convention.lower() in ('std', 'fwhm'): sigma_function = width_function else: - msg = 'width_convention must be "std" or "fwhm".' + msg = format_error( + f'Invalid width convention: {width_convention}.', + fix='`width_convention` must be "std" or "fwhm".', + ) raise ValueError(msg) widths = sigma_function(x) @@ -107,8 +104,7 @@ def sigma_function(x: Quantity) -> Quantity: return width_interpolated_broadening(bins, x, widths, weights.magnitude, adaptive_error=adaptive_error, - shape=shape, - fit=fit) * weights_unit + shape=shape) * weights_unit def width_interpolated_broadening( @@ -118,7 +114,6 @@ def width_interpolated_broadening( weights: np.ndarray, adaptive_error: float, shape: KernelShape = 'gauss', - fit: ErrorFit = 'cheby-log', ) -> Quantity: """ Uses a fast, approximate method to broaden a spectrum @@ -147,10 +142,6 @@ def width_interpolated_broadening( shape Select kernel shape. Widths will correspond to sigma or gamma parameters respectively. - fit - Select parametrisation of kernel width spacing to adaptive_error. - 'cheby-log' is recommended: for shape 'gauss', 'cubic' is also - available. Returns ------- @@ -164,8 +155,7 @@ def width_interpolated_broadening( widths.to(bins.units).magnitude, weights, adaptive_error, - shape=shape, - fit=fit) / bins.units + shape=shape) / bins.units def _lorentzian(x: np.ndarray, gamma: np.ndarray) -> np.ndarray: @@ -173,24 +163,12 @@ def _lorentzian(x: np.ndarray, gamma: np.ndarray) -> np.ndarray: def _get_spacing(error, - shape: KernelShape = 'gauss', - fit: ErrorFit = 'cheby-log'): + shape: KernelShape = 'gauss'): """ Determine suitable spacing value for mode_width given accepted error level Coefficients have been fitted to plots of error vs spacing value """ - - if fit == 'cubic' and shape == 'gauss': - return np.polyval([612.7, -122.7, 15.40, 1.0831], error) - - if fit != 'cheby-log': - msg = dedent_and_fill(f""" - Fit "{fit}" is not available for shape "{shape}". The "cheby-log" - fit is recommended for "gauss" and "Lorentz" shapes.' - """) - raise ValueError(msg) - if shape == 'lorentz': cheby = Chebyshev( [1.26039672, 0.39900457, 0.20392176, 0.08602507, @@ -209,9 +187,9 @@ def _get_spacing(error, log_error = np.log10(error) if not safe_domain[0] < log_error < safe_domain[1]: - msg = ( - 'Target error is out of fit range; value must lie ' - f'in range {np.power(10, safe_domain)}.' + msg = format_error( + f'Target error ({error}) is out of fit range.', + fix=f'Value must lie in range {np.power(10, safe_domain)}.', ) raise ValueError(msg) return cheby(log_error) @@ -224,7 +202,7 @@ def _width_interpolated_broadening( weights: np.ndarray, adaptive_error: float, shape: KernelShape = 'gauss', - fit: ErrorFit = 'cheby-log') -> np.ndarray: +) -> np.ndarray: """ Broadens a spectrum using a variable-width kernel, taking the same arguments as `variable_width` but expects arrays with @@ -234,7 +212,7 @@ def _width_interpolated_broadening( x = np.ravel(x) widths = np.ravel(widths) weights = np.ravel(weights) - spacing = _get_spacing(adaptive_error, shape=shape, fit=fit) + spacing = _get_spacing(adaptive_error, shape=shape) # bins should be regularly spaced, check that this is the case and # raise a warning if not diff --git a/euphonic/cli/brille_convergence.py b/euphonic/cli/brille_convergence.py index 06b9d563a..a81559c1a 100644 --- a/euphonic/cli/brille_convergence.py +++ b/euphonic/cli/brille_convergence.py @@ -18,20 +18,25 @@ load_data_from_file, ) from euphonic.sampling import recurrence_sequence +from euphonic.util import format_error def main(params: list[str] | None = None) -> None: args = get_args(get_parser(), params) params = vars(args) + del params['use_brille'] # This tool _always_ uses brille + match params['energy_broadening']: case int(value) | float(value) | [int(value)] | [float(value)]: params['energy_broadening'] = value case None: pass case _: - msg = ('This script only allows a fixed-width broadening ' - '(i.e. --energy-broadening should be a single value)') + msg = format_error( + 'This script only allows a fixed-width broadening.', + fix='--energy-broadening should be a single value.', + ) raise ValueError(msg) check_brille_settings(**params) @@ -40,7 +45,6 @@ def main(params: list[str] | None = None) -> None: def check_brille_settings( filename: Path | str, npts: int = 500, - use_brille: bool = True, # noqa: ARG001 # Removal scheduled for v2 brille_grid_type: str = 'trellis', brille_npts: int = 5000, brille_npts_density: int | None = None, @@ -56,9 +60,10 @@ def check_brille_settings( fc = load_data_from_file(filename) if not isinstance(fc, ForceConstants): - msg = ( - 'Force constants are required to use the ' - 'euphonic-check-brille-settings tool' + msg = format_error( + ('Force constants are required to use the ' + 'euphonic-check-brille-settings tool'), + fix='Use a data file with force constants available.', ) raise TypeError(msg) diff --git a/euphonic/cli/dispersion.py b/euphonic/cli/dispersion.py index 799874b12..efb8a5136 100644 --- a/euphonic/cli/dispersion.py +++ b/euphonic/cli/dispersion.py @@ -5,6 +5,7 @@ from euphonic import ForceConstants, QpointPhononModes, Spectrum1D from euphonic.plot import plot_1d from euphonic.styles import base_style +from euphonic.util import format_error from euphonic.writers.phonon_website import write_phonon_website_json from .utils import ( @@ -32,9 +33,13 @@ def main(params: list[str] | None = None) -> None: if not frequencies_only and not isinstance( data, (ForceConstants, QpointPhononModes)): - msg = 'Eigenvectors are required to use "--reorder" option' - raise TypeError( - msg) + + msg = format_error( + 'Eigenvectors are required to use "--reorder" option.', + fix=('Use a data file which contains ' + 'eigenvectors or force constants.'), + ) + raise TypeError(msg) if isinstance(data, ForceConstants): print('Getting band path...') diff --git a/euphonic/cli/dos.py b/euphonic/cli/dos.py index a959295cb..46d19e8a9 100644 --- a/euphonic/cli/dos.py +++ b/euphonic/cli/dos.py @@ -7,7 +7,7 @@ from euphonic import ForceConstants, QpointPhononModes, ureg from euphonic.plot import plot_1d from euphonic.styles import base_style -from euphonic.util import dedent_and_fill, mode_gradients_to_widths, mp_grid +from euphonic.util import format_error, mode_gradients_to_widths, mp_grid from .utils import ( _arrange_pdos_groups, @@ -34,31 +34,46 @@ def main(params: list[str] | None = None) -> None: if not frequencies_only and not isinstance( data, (QpointPhononModes, ForceConstants)): - msg = ( + msg = format_error( + 'No eigenvectors found', + reason=( 'Eigenvectors are required to use "--pdos" or ' 'any "--weighting" option other than plain DOS' + ), + fix=('Use a data file which contains ' + 'eigenvectors or force constants.'), ) raise TypeError(msg) if args.adaptive and not isinstance(data, ForceConstants): - msg = 'Force constants are required to use --adaptive option' + msg = format_error( + 'No force constants found.', + reason=('The --adaptive option requires ' + 'mode gradients to be calculated.'), + fix='Use a data file that contains force constants.', + ) raise TypeError(msg) if (args.energy_broadening and args.adaptive and len(args.energy_broadening) == 1): if args.adaptive_scale is not None: - msg = dedent_and_fill(""" - Adaptive scale factor was specified twice; use either - --adaptive-scale or --energy-broadening. To add a fixed width - to adaptive broadening, use --instrument-broadening.""") + msg = format_error( + 'Adaptive scale factor was specified twice.', + fix=""" + Use either --adaptive-scale or --energy-broadening. + + To add a fixed width to adaptive broadening, + use --instrument-broadening.""", + ) raise ValueError(msg) args.adaptive_scale = args.energy_broadening[0] elif args.energy_broadening: if args.inst_broadening is not None: - msg = ( - 'Broadening width was specified twice; use either' - '--instrument-broadening or --energy-broadening.' + msg = format_error( + 'Broadening width was specified twice.', + fix=('Use either --instrument-broadening ' + 'or --energy-broadening.'), ) raise ValueError(msg) args.inst_broadening = args.energy_broadening @@ -106,8 +121,7 @@ def main(params: list[str] | None = None) -> None: kwargs = {'mode_widths': mode_widths, 'adaptive_method': args.adaptive_method, - 'adaptive_error': args.adaptive_error, - 'adaptive_error_fit': args.adaptive_fit} + 'adaptive_error': args.adaptive_error} if args.weighting == 'dos' and args.pdos is None: dos = modes.calculate_dos(ebins, **kwargs) @@ -128,8 +142,7 @@ def energy_broadening_func(x): dos = dos.broaden(x_width=energy_broadening_func, shape=args.shape, method='convolve', - width_interpolation_error=args.adaptive_error, - width_fit='cheby-log') + width_interpolation_error=args.adaptive_error) elif args.inst_broadening: # Fixed-width broadening diff --git a/euphonic/cli/intensity_map.py b/euphonic/cli/intensity_map.py index 57bc840b6..7af391de9 100644 --- a/euphonic/cli/intensity_map.py +++ b/euphonic/cli/intensity_map.py @@ -6,7 +6,7 @@ from euphonic import ForceConstants, QpointPhononModes, ureg import euphonic.plot from euphonic.styles import base_style -from euphonic.util import dedent_and_fill, get_qpoint_labels +from euphonic.util import format_error, get_qpoint_labels from .utils import ( _bands_from_force_constants, @@ -33,16 +33,23 @@ def main(params: list[str] | None = None) -> None: if (not frequencies_only and not isinstance(data, (QpointPhononModes, ForceConstants)) ): - msg = 'Eigenvectors are required to use "--weighting coherent" option' + msg = format_error( + 'Eigenvectors are required to use "--weighting coherent" option.', + fix='Use a data file containing eigenvectors or force constants.', + ) raise TypeError(msg) if (args.weighting.lower() == 'coherent' and args.temperature is not None and not isinstance(data, ForceConstants) ): - msg = dedent_and_fill(""" - Force constants data is required to generate the Debye-Waller - factor. Leave "--temperature" unset if plotting precalculated - phonon modes.""") + msg = format_error( + 'Force constants required to generate the Debye-Waller factor.', + fix=""" + Use a data file containing force constants. + + Leave "--temperature" unset if plotting precalculated phonon modes. + """, + ) raise TypeError(msg) q_spacing = _get_q_distance(args.length_unit, args.q_spacing) diff --git a/euphonic/cli/optimise_dipole_parameter.py b/euphonic/cli/optimise_dipole_parameter.py index 0bddcca71..9359a7188 100644 --- a/euphonic/cli/optimise_dipole_parameter.py +++ b/euphonic/cli/optimise_dipole_parameter.py @@ -15,6 +15,7 @@ from euphonic import ForceConstants from euphonic.cli.utils import _get_cli_parser, get_args, load_data_from_file +from euphonic.util import format_error # Formatting string for timing output TEXT: VALUE UNITS TIMING_TEMPLATE = '{:20s}: {:3.2f} {:s}\n' @@ -81,9 +82,12 @@ def calculate_optimum_dipole_parameter( fc = load_data_from_file(filename) if not isinstance(fc, ForceConstants): - msg = ( - 'Force constants are required to use the ' - 'euphonic-optimise-dipole-parameter tool' + msg = format_error( + ( + 'Force constants are required to use the ' + 'euphonic-optimise-dipole-parameter tool' + ), + fix='Use a data file containing force constants', ) raise TypeError(msg) # Only warn rather than error - although not designed for it diff --git a/euphonic/cli/powder_map.py b/euphonic/cli/powder_map.py index 713567d96..a68db8441 100644 --- a/euphonic/cli/powder_map.py +++ b/euphonic/cli/powder_map.py @@ -31,7 +31,7 @@ ) from euphonic.spectra import apply_kinematic_constraints from euphonic.styles import base_style, intensity_widget_style -import euphonic.util +from euphonic.util import format_error # Dummy tqdm function if tqdm progress bars unavailable try: @@ -133,15 +133,19 @@ def main(params: list[str] | None = None) -> None: fc = load_data_from_file(args.filename, verbose=True) if not isinstance(fc, ForceConstants): - msg = ( - 'Force constants are required to use the ' - 'euphonic-powder-map tool' + msg = format_error( + ('Force constants are required to ' + 'use the euphonic-powder-map tool.'), + fix='Use a data file containing force constants.', ) raise TypeError(msg) if args.pdos is not None and args.weighting == 'coherent': - msg = ( - '"--pdos" is only compatible with ' - '"--weighting" options that include dos' + msg = format_error( + 'Incompatible options specified.', + reason=('Coherent scattering cannot be decomposed ' + 'to linear single-atom contributions.'), + fix=('Use --pdos with another weighting scheme, ' + 'or use "coherent" weighting without --pdos.'), ) raise ValueError(msg) diff --git a/euphonic/cli/utils.py b/euphonic/cli/utils.py index a4107fb65..69ea63524 100644 --- a/euphonic/cli/utils.py +++ b/euphonic/cli/utils.py @@ -28,7 +28,12 @@ Spectrum1DCollection, ureg, ) -from euphonic.util import dedent_and_fill, mp_grid, spglib_new_errors +from euphonic.util import ( + dedent_and_fill, + format_error, + mp_grid, + spglib_new_errors, +) def _load_euphonic_json(filename: str | os.PathLike, @@ -44,7 +49,10 @@ def _load_euphonic_json(filename: str | os.PathLike, return QpointPhononModes.from_json_file(filename) return QpointFrequencies.from_json_file(filename) - msg = 'Could not identify Euphonic data in JSON file.' + msg = format_error( + f'Could not identify Euphonic data in JSON file ({filename}).', + fix='Ensure JSON file contains "force_constants" or "frequencies".', + ) raise ValueError(msg) @@ -80,9 +88,14 @@ def _load_phonopy_file(filename: str | os.PathLike, phonopy_kwargs['summary_name'] = 'phonopy.yaml' phonopy_kwargs['fc_name'] = path.name else: - msg = ( - 'Phonopy force_constants.hdf5 file ' - 'must be accompanied by phonopy.yaml' + msg = format_error( + 'Missing phonopy.yaml.', + reason = ( + 'Phonopy force_constants.hdf5 file ' + 'must be accompanied by information ' + 'about atomic masses, supercell, etc.' + ), + fix='Ensure phonopy.yaml provided.', ) raise ValueError(msg) elif path.suffix in ('.yaml', '.yml'): @@ -153,13 +166,17 @@ def load_data_from_file(filename: str | os.PathLike, elif path.suffix in phonopy_suffixes: data = _load_phonopy_file(path, frequencies_only) else: - msg = dedent_and_fill(f""" - File format was not recognised. CASTEP force constants data for + msg = format_error( + f'File format ({path.suffix}) not recognised.', + reason=f""" + CASTEP force constants data for import should have extension from {castep_fc_suffixes}, CASTEP phonon mode data for import should have extension '{castep_qpm_suffixes}', data from Phonopy should have extension from {phonopy_suffixes}, data from Euphonic should have extension - '.json'.""") + '.json'.""", + fix='Ensure file format in known formats.', + ) raise ValueError(msg) if verbose: print(f'{data.__class__.__name__} data was loaded') @@ -213,9 +230,11 @@ def _get_q_distance(length_unit_string: str, q_distance: float) -> Quantity: try: length_units = ureg(length_unit_string) except UndefinedUnitError as err: - msg = ( - 'Length unit not known. Euphonic uses Pint for units. Try ' - "'angstrom' or 'bohr'. Metric prefixes are also allowed, e.g 'nm'." + msg = format_error( + 'Length unit not known', + reason='Euphonic uses Pint for units.', + fix=("Try 'angstrom' or 'bohr'. " + "Metric prefixes are also allowed, e.g 'nm'."), ) raise ValueError(msg) from err recip_length_units = 1 / length_units @@ -242,9 +261,9 @@ def _get_energy_bins( if emax is None: emax = np.max(modes.frequencies.magnitude) * headroom if emin >= emax: - msg = ( - 'Maximum energy should be greater than minimum. ' - 'Check --e-min and --e-max arguments.' + msg = format_error( + 'Maximum energy should be greater than minimum.', + fix='Check --e-min and --e-max arguments.', ) raise ValueError(msg) return np.linspace(emin, emax, n_ebins) * modes.frequencies.units @@ -437,7 +456,10 @@ def _get_pdos_weighting(cl_arg_weighting: str) -> str | None: else: idx = cl_arg_weighting.rfind('-') if idx == -1: - msg = f'Unexpected weighting "{cl_arg_weighting}"' + msg = format_error( + f'Unexpected weighting "{cl_arg_weighting}"', + fix='Check weighting argument. Should be e.g. "coherent-dos".', + ) raise ValueError(msg) pdos_weighting = cl_arg_weighting[:idx] return pdos_weighting @@ -568,7 +590,11 @@ def _get_cli_parser(features: Collection[str] = {}, 'Accepted formats: .yaml, force_constants.hdf5 (Phonopy); ' '.castep_bin, .check (Castep); .json (Euphonic).') else: - msg = 'No band-data-only tools have been defined.' + msg = format_error( + 'Invalid option requested.', + reason='No band-data-only tools have been defined.', + fix='Check the requested features are correct.', + ) raise ValueError(msg) sections['file'].add_argument('filename', type=str, help=filename_doc) @@ -781,16 +807,6 @@ def _get_cli_parser(features: Collection[str] = {}, dest='adaptive_scale', help='Scale factor applied to adaptive broadening width', ) - section.add_argument( - '--adaptive-fit', type=str, choices=['cubic', 'cheby-log'], - default='cubic', dest='adaptive_fit', - help=('Select parametrisation for fast adaptive broadening' - '. "cheby-log" is generally recommended, "cubic" is ' - 'default retained for backward-compatibility. This ' - 'only applies when adaptive broadening is used; if ' - 'variable-width instrument broadening is used alone ' - 'then "cheby-log" will be used.' ), - ) section.add_argument( '--instrument-broadening', type=float, nargs='+', default=None, dest='inst_broadening', help=ib_help) @@ -811,7 +827,11 @@ def _get_cli_parser(features: Collection[str] = {}, choices=('gauss', 'lorentz'), help='The broadening shape') else: - msg = '"adaptive-broadening" cannot be applied without "ebins"' + msg = format_error( + 'Missing "ebins" argument.', + reason='"adaptive-broadening" cannot be used without "ebins".', + fix='Include "ebins" in features.', + ) raise ValueError(msg) if {'q-e', 'mp-grid'}.intersection(features): diff --git a/euphonic/crystal.py b/euphonic/crystal.py index 1c308c5f9..d363955d7 100644 --- a/euphonic/crystal.py +++ b/euphonic/crystal.py @@ -14,7 +14,11 @@ _process_dict, ) from euphonic.ureg import Quantity, ureg -from euphonic.util import _cell_vectors_to_volume, _get_unique_elems_and_idx +from euphonic.util import ( + _cell_vectors_to_volume, + _get_unique_elems_and_idx, + format_error, +) from euphonic.validate import _check_constructor_inputs, _check_unit_conversion @@ -245,9 +249,10 @@ def get_symmetry_equivalent_atoms( # For some reason this can't always be reproduced symm = spglib.get_symmetry(self.to_spglib_cell(), symprec=symprec) if symm is None: - msg = ( - f'spglib.get_symmetry returned None with ' - f'symprec={symprec}. Try increasing tol' + msg = format_error( + 'spglib.get_symmetry unable to determine symmetry.', + reason=f'Tolerance ({symprec}) may be too low.', + fix='Try increasing tol.', ) raise RuntimeError(msg) n_ops = len(symm['rotations']) @@ -277,11 +282,16 @@ def get_symmetry_equivalent_atoms( f'symmetry op {op_idx}. Rotation ' f'{symm["rotations"][op_idx]} translation ' f'{symm["translations"][op_idx]}') - if len(equiv_idx_op) == 0: - msg = f'No equivalent atom found {err_info}' - raise RuntimeError(msg) - if len(equiv_idx_op) > 1: - msg = f'Multiple equivalent atoms found {err_info}' + if len(equiv_idx_op) != 1: + typ = ('No' + if len(equiv_idx_op) == 0 else + 'Multiple') + msg = format_error( + f'{typ} equivalent atoms.', + reason=(f'{typ} equivalent atoms found ' + f'{err_info}.'), + fix='Check structure or tighten tolerance.', + ) raise RuntimeError(msg) equiv_atoms[:, i] = idx[equiv_idx[1]] diff --git a/euphonic/force_constants.py b/euphonic/force_constants.py index 2e5506cbd..c43577eb2 100644 --- a/euphonic/force_constants.py +++ b/euphonic/force_constants.py @@ -30,6 +30,7 @@ from euphonic.util import ( _get_supercell_relative_idx, dedent_and_fill, + format_error, get_all_origins, is_gamma, ) @@ -1517,7 +1518,10 @@ def _find_acoustic_modes(self, dyn_mat: np.ndarray, sc_mass = 1.0*n_atoms # Check number of acoustic modes if np.sum(c_of_m_disp_sq > sensitivity * sc_mass) < 3: - msg = 'Could not find 3 acoustic modes' + msg = format_error( + 'Could not find 3 acoustic modes.', + fix='Check convergence of phonon calculations.', + ) raise NotEnoughAcousticModesError(msg) from None # Find idx of acoustic modes (3 largest c of m displacements) ac_i = np.argsort(c_of_m_disp_sq)[-3:] diff --git a/euphonic/plot.py b/euphonic/plot.py index 8b5ae9dfa..ec9a2f221 100644 --- a/euphonic/plot.py +++ b/euphonic/plot.py @@ -6,7 +6,7 @@ from euphonic.spectra import Spectrum1D, Spectrum1DCollection, Spectrum2D from euphonic.ureg import Quantity -from euphonic.util import dedent_and_fill, zips +from euphonic.util import format_error, zips if TYPE_CHECKING: from matplotlib.lines import Line2D @@ -19,13 +19,16 @@ import matplotlib.pyplot as plt except ModuleNotFoundError as err: - err_msg = dedent_and_fill(""" - Cannot import Matplotlib for plotting (maybe Matplotlib is - not installed?). To install Euphonic's optional Matplotlib + err_msg = format_error( + 'Cannot import matplotlib for plotting.', + reason='Maybe matplotlib is not installed.', + fix=""" + To install Euphonic's optional Matplotlib dependency, try: pip install euphonic[matplotlib] - """) + """, + ) raise ModuleNotFoundError(err_msg) from err @@ -61,15 +64,20 @@ def plot_1d_to_axis(spectra: Spectrum1D | Spectrum1DCollection, try: assert isinstance(spectra, Spectrum1DCollection) except AssertionError: - msg = 'spectra should be a Spectrum1D or Spectrum1DCollection' + msg = format_error( + f'Invalid type for spectra ({type(spectra).__name__}).', + reason='spectra should be a Spectrum1D or Spectrum1DCollection.', + fix='Ensure spectrum is a Spectrum1D or Spectrum1DCollection.', + ) raise TypeError(msg) from None if isinstance(labels, str): labels = [labels] if labels is not None and len(labels) != len(spectra): - msg = ( - f'The length of labels (got {len(labels)}) should be the ' - f'same as the number of lines to plot (got {len(spectra)})' + msg = format_error( + (f'Mismatched length for labels ({len(labels)}) ' + f'and spectra ({len(spectra)}).'), + fix='Ensure one label per spectrum.', ) raise ValueError(msg) @@ -181,19 +189,22 @@ def plot_1d(spectra: OneDSpectrumOrSpectra, else: # Check units are consistent for spectrum in spectra[1:]: + invalid = '' if spectrum.x_data_unit != spectra[0].x_data_unit: - msg = ( - 'Something went wrong: x data units are not ' - 'consistent between spectrum subplots.' - ) - raise ValueError(msg) + invalid += 'x' if spectrum.y_data_unit != spectra[0].y_data_unit: - msg = ( - 'Something went wrong: y data units are not ' - 'consistent between spectrum subplots.' + invalid += 'y' + if invalid: + msg = format_error( + 'Inconsistent units.', + reason=(f'{" and ".join(invalid)} data units ' + 'are not consistent ' + 'between spectrum subplots.'), + fix='Ensure units are the same for each spectrum', ) raise ValueError(msg) + gridspec_kw = _get_gridspec_kw(spectra) fig, subplots = plt.subplots(1, len(spectra), sharey=True, gridspec_kw=gridspec_kw, squeeze=False) diff --git a/euphonic/powder.py b/euphonic/powder.py index 9fdc382e9..7365da127 100644 --- a/euphonic/powder.py +++ b/euphonic/powder.py @@ -1,6 +1,6 @@ """Functions for averaging spectra in spherical q bins""" -from typing import Literal +from typing import Literal, get_args import numpy as np @@ -14,7 +14,14 @@ Spectrum1D, Spectrum1DCollection, ) -from euphonic.util import RNG, get_reference_data, mp_grid, rng +from euphonic.util import ( + RNG, + comma_join, + format_error, + get_reference_data, + mp_grid, + rng, +) SphericalSamplingOptions = Literal['golden', 'sphere-projected-grid', @@ -25,11 +32,12 @@ def sample_sphere_dos(fc: ForceConstants, mod_q: Quantity, + *, sampling: SphericalSamplingOptions = 'golden', npts: int = 1000, jitter: bool = False, - energy_bins: Quantity = None, rng: RNG = rng, + energy_bins: Quantity | None = None, **calc_modes_args, ) -> Spectrum1D: """ @@ -78,11 +86,11 @@ def sample_sphere_dos(fc: ForceConstants, jitter For non-random sampling schemes, apply an additional random displacement to each point. + rng + Numpy random Generator for random sampling and jitter energy_bins Preferred energy bin edges. If not provided, will setup 1000 bins (1001 bin edges) from 0 to 1.05 * [max energy] - rng - Numpy random Generator for random sampling and jitter **calc_modes_args other keyword arguments (e.g. 'use_c') will be passed to ForceConstants.calculate_qpoint_phonon_modes() @@ -117,13 +125,14 @@ def sample_sphere_dos(fc: ForceConstants, def sample_sphere_pdos( fc: ForceConstants, mod_q: Quantity, + *, sampling: SphericalSamplingOptions = 'golden', npts: int = 1000, jitter: bool = False, - energy_bins: Quantity = None, + rng: RNG = rng, + energy_bins: Quantity | None = None, weighting: str | None = None, cross_sections: str | dict[str, Quantity] = 'BlueBook', - rng: RNG = rng, **calc_modes_args, ) -> Spectrum1DCollection: """ @@ -172,6 +181,8 @@ def sample_sphere_pdos( jitter For non-random sampling schemes, apply an additional random displacement to each point. + rng + Numpy random Generator for random sampling and jitter energy_bins Preferred energy bin edges. If not provided, will setup 1000 bins (1001 bin edges) from 0 to 1.05 * [max energy] @@ -199,9 +210,6 @@ def sample_sphere_pdos( appropriate units, e.g:: {'La': 8.0*ureg('barn'), 'Zr': 9.5*ureg('barn')} - rng - Numpy random Generator for random sampling and jitter - **calc_modes_args other keyword arguments (e.g. 'use_c') will be passed to ForceConstants.calculate_qpoint_phonon_modes() @@ -234,14 +242,16 @@ def sample_sphere_pdos( def sample_sphere_structure_factor( fc: ForceConstants, mod_q: Quantity, + *, dw: DebyeWaller = None, dw_spacing: Quantity = Quantity(0.025, '1/angstrom'), temperature: Quantity | None = Quantity(273., 'K'), sampling: SphericalSamplingOptions = 'golden', - npts: int = 1000, jitter: bool = False, + npts: int = 1000, + jitter: bool = False, + rng: RNG = rng, energy_bins: Quantity = None, scattering_lengths: str | dict[str, Quantity] = 'Sears1992', - rng: RNG = rng, **calc_modes_args, ) -> Spectrum1D: """Sample structure factor, averaging over a sphere of constant |q| @@ -301,6 +311,8 @@ def sample_sphere_structure_factor( jitter For non-random sampling schemes, apply an additional random displacement to each point. + rng + Numpy random Generator for random sampling and jitter energy_bins Preferred energy bin edges. If not provided, will setup 1000 bins (1001 bin edges) from 0 to 1.05 * [max energy] @@ -309,8 +321,6 @@ def sample_sphere_structure_factor( string is provided, this selects coherent scattering lengths from reference data by setting the 'label' argument of the euphonic.util.get_reference_data() function. - rng - Numpy random Generator for random sampling and jitter **calc_modes_args other keyword arguments (e.g. 'use_c') will be passed to ForceConstants.calculate_qpoint_phonon_modes() @@ -322,21 +332,24 @@ def sample_sphere_structure_factor( """ if isinstance(scattering_lengths, str): - scattering_lengths = get_reference_data( + scattering_lengths: dict = get_reference_data( physical_property='coherent_scattering_length', - collection=scattering_lengths) # type: dict + collection=scattering_lengths) if temperature is not None: if (dw is None): dw_qpts = mp_grid(fc.crystal.get_mp_grid_spec(dw_spacing)) dw_phonons = fc.calculate_qpoint_phonon_modes(dw_qpts, **calc_modes_args) - dw = dw_phonons.calculate_debye_waller(temperature, - ) # type: DebyeWaller + dw: DebyeWaller = dw_phonons.calculate_debye_waller(temperature) elif not np.isclose(dw.temperature, temperature): - msg = ( - 'Temperature argument is not consistent with ' - 'temperature stored in DebyeWaller object.' + msg = format_error( + 'Inconsistent temperature.', + reason=( + 'Temperature argument is not consistent with ' + 'temperature stored in DebyeWaller object.' + ), + fix='Ensure consistency between values.', ) raise ValueError(msg) @@ -352,8 +365,8 @@ def sample_sphere_structure_factor( qpts_frac = _qpts_cart_to_frac(qpts_cart, fc.crystal) - phonons = fc.calculate_qpoint_phonon_modes(qpts_frac, **calc_modes_args, - ) # type: QpointPhononModes + phonons: QpointPhononModes = fc.calculate_qpoint_phonon_modes( + qpts_frac, **calc_modes_args) if energy_bins is None: energy_bins = _get_default_bins(phonons) @@ -450,7 +463,11 @@ def _get_qpts_sphere(npts: int, case 'random-sphere': return np.asarray(list(random_sphere(npts, rng=rng))) - msg = f'Sampling method "{sampling}" is unknown.' + msg = format_error( + f'Unknown sampling method ({sampling}).', + fix=('Valid sampling methods are: ' + f'{comma_join(get_args(SphericalSamplingOptions))}.'), + ) raise ValueError(msg) diff --git a/euphonic/qpoint_frequencies.py b/euphonic/qpoint_frequencies.py index 117685de3..5437f6b0d 100644 --- a/euphonic/qpoint_frequencies.py +++ b/euphonic/qpoint_frequencies.py @@ -4,7 +4,7 @@ import numpy as np -from euphonic.broadening import ErrorFit, _width_interpolated_broadening +from euphonic.broadening import _width_interpolated_broadening from euphonic.crystal import Crystal from euphonic.io import ( _obj_from_json_file, @@ -15,7 +15,12 @@ from euphonic.readers import castep, phonopy from euphonic.spectra import Spectrum1D, Spectrum1DCollection, Spectrum2D from euphonic.ureg import Quantity, ureg -from euphonic.util import _calc_abscissa, get_qpoint_labels +from euphonic.util import ( + _calc_abscissa, + comma_join, + format_error, + get_qpoint_labels, +) from euphonic.validate import _check_constructor_inputs, _check_unit_conversion AdaptiveMethod = Literal['reference', 'fast'] @@ -103,7 +108,6 @@ def calculate_dos( mode_widths_min: Quantity = Quantity(0.01, 'meV'), adaptive_method: AdaptiveMethod = 'reference', adaptive_error: float = 0.01, - adaptive_error_fit: ErrorFit = 'cubic', ) -> Spectrum1D: """ Calculates a density of states, in units of modes per atom per @@ -129,10 +133,6 @@ def calculate_dos( Scalar float. Acceptable error for gaussian approximations when using the fast adaptive method, defined as the absolute difference between the areas of the true and approximate gaussians - adaptive_error_fit - Select parametrisation of kernel width spacing to adaptive_error. - 'cheby-log' is recommended: for backward-compatibilty, 'cubic' is - the default. Returns ------- @@ -158,8 +158,7 @@ def calculate_dos( dos = self._calculate_dos(dos_bins, mode_widths=mode_widths, mode_widths_min=mode_widths_min, adaptive_method=adaptive_method, - adaptive_error=adaptive_error, - adaptive_error_fit=adaptive_error_fit) + adaptive_error=adaptive_error) return Spectrum1D(dos_bins, dos) def _calculate_dos( @@ -170,7 +169,6 @@ def _calculate_dos( mode_weights: np.ndarray | None = None, adaptive_method: AdaptiveMethod = 'reference', adaptive_error: float = 0.01, - adaptive_error_fit: ErrorFit = 'cubic', q_idx: int | None = None, ) -> Quantity: """ @@ -192,9 +190,10 @@ def _calculate_dos( adaptive_method_options = ['reference', 'fast'] if adaptive_method not in adaptive_method_options: - msg = ( - f'Invalid value for adaptive_method, got {adaptive_method}, ' - f'should be one of {adaptive_method_options}' + msg = format_error( + f'Invalid value for adaptive_method ({adaptive_method}).', + fix=('adaptive_method should be one of: ' + f'{comma_join(adaptive_method_options)}.'), ) raise ValueError(msg) @@ -235,8 +234,7 @@ def _calculate_dos( dos = _width_interpolated_broadening(dos_bins_calc, freqs, mode_widths, combined_weights, - adaptive_error, - fit=adaptive_error_fit) + adaptive_error) else: bin_idx = np.digitize(freqs, dos_bins_calc) # Create DOS with extra bin either side, for any points diff --git a/euphonic/qpoint_phonon_modes.py b/euphonic/qpoint_phonon_modes.py index 8a3f63f3f..945338c76 100644 --- a/euphonic/qpoint_phonon_modes.py +++ b/euphonic/qpoint_phonon_modes.py @@ -7,7 +7,6 @@ import numpy as np -from euphonic.broadening import ErrorFit from euphonic.crystal import Crystal from euphonic.debye_waller import DebyeWaller from euphonic.io import _obj_from_json_file, _obj_to_dict, _process_dict @@ -16,7 +15,13 @@ from euphonic.spectra import Spectrum1DCollection from euphonic.structure_factor import StructureFactor from euphonic.ureg import Quantity, ureg -from euphonic.util import direction_changed, get_reference_data, is_gamma +from euphonic.util import ( + comma_join, + direction_changed, + format_error, + get_reference_data, + is_gamma, +) from euphonic.validate import _check_constructor_inputs @@ -251,9 +256,10 @@ def calculate_structure_factor( elif isinstance(scattering_lengths, dict): scattering_length_data = scattering_lengths else: - msg = ( - f'Unexpected type for scattering_lengths, should be str ' - f'or dict, got {type(scattering_lengths)}' + msg = format_error( + 'Unexpected type for scattering_lengths ' + f'({type(scattering_lengths).__name__}).', + fix='scattering_lengths should be str or dict.', ) raise TypeError(msg) @@ -282,10 +288,12 @@ def calculate_structure_factor( if dw: temperature = dw.temperature if dw.crystal.n_atoms != self.crystal.n_atoms: - msg = ( - 'The DebyeWaller object used as dw is not ' - 'compatible with the QPointPhononModes object (they' - ' have a different number of atoms)' + msg = format_error( + 'Incompatible types (mismatched atom counts).', + reason=( + 'The DebyeWaller object used as dw is not ' + 'compatible with the QPointPhononModes object.'), + fix='Ensure both objects have the same number of atoms.', ) raise ValueError(msg) dw_factor = np.exp(-np.einsum('jkl,ik,il->ij', @@ -438,7 +446,6 @@ def calculate_pdos( mode_widths_min: Quantity = Quantity(0.01, 'meV'), adaptive_method: AdaptiveMethod = 'reference', adaptive_error: float | None = 0.01, - adaptive_error_fit: ErrorFit = 'cubic', weighting: str | None = None, cross_sections: str | dict[str, Quantity] = 'BlueBook', ) -> Spectrum1DCollection: @@ -467,10 +474,6 @@ def calculate_pdos( Scalar float. Acceptable error for gaussian approximations when using the fast adaptive method, defined as the absolute difference between the areas of the true and approximate gaussians - adaptive_error_fit - Select parametrisation of kernel width spacing to adaptive_error - when using fast approximate method. 'cheby-log' is recommended: - for backward-compatibilty, 'cubic' is the default. weighting One of {'coherent', 'incoherent', 'coherent-plus-incoherent'}. If provided, produces a neutron-weighted DOS, weighted by @@ -546,9 +549,10 @@ def calculate_pdos( weighting_opts = [None, 'coherent', 'incoherent', 'coherent-plus-incoherent'] if weighting not in weighting_opts: - msg = ( - f'Invalid value for weighting, got {weighting}, should be one ' - f'of {weighting_opts}' + msg = format_error( + f'Invalid weighting ({weighting}).', + fix=('weighting should be one of ' + f'{comma_join(weighting_opts)}.'), ) raise ValueError(msg) @@ -566,9 +570,10 @@ def calculate_pdos( elif isinstance(cross_sections, Mapping): cross_sections_data = [cross_sections] else: - msg = ( - f'Unexpected type for cross_sections, expected ' - f'str or dict, got {type(cross_sections)}' + msg = format_error( + 'Invalid type for cross_sections ' + f'({type(cross_sections).__name__}).', + fix='cross_sections should be str or dict.', ) raise TypeError(msg) @@ -581,10 +586,11 @@ def calculate_pdos( # Account for cross sections in different, or invalid, units ex_units = '[length]**2' if not cs[0].check(ex_units): - msg = ( - f'Unexpected dimensionality in cross_sections units, ' - f'expected {ex_units}, got {cs[0].dimensionality!s}' - ) + msg = format_error( + 'Unexpected dimensions in cross_sections units ' + f'({cs[0].dimensionality}).', + fix=f'cross_sections should have units of {ex_units}.', + ) raise ValueError(msg) cs = [x.to('mbarn') for x in cs] else: @@ -601,7 +607,6 @@ def calculate_pdos( mode_widths_min=mode_widths_min, adaptive_method=adaptive_method, adaptive_error=adaptive_error, - adaptive_error_fit=adaptive_error_fit, mode_weights=evec_weights[:, :, i]) if cs is not None: # Neutron weighted DOS is per atom of sample diff --git a/euphonic/readers/castep.py b/euphonic/readers/castep.py index 2883fe5fc..3deba8022 100644 --- a/euphonic/readers/castep.py +++ b/euphonic/readers/castep.py @@ -14,7 +14,7 @@ from packaging.version import Version from euphonic.ureg import ureg -from euphonic.util import dedent_and_fill +from euphonic.util import format_error def read_phonon_dos_data( @@ -246,9 +246,13 @@ def read_phonon_data( zero_indices = np.asarray(list(intersection), dtype = int) weights[zero_indices] = 0 else: - msg = ( - 'Found multiple non-split blocks for q-point {qpt_id},' - ' cannot determine which to use.' + msg = format_error( + f'Cannot determine blocks for q-point ({qpt_id}).', + reason=( + 'Found multiple non-split blocks for q-point,' + ' cannot determine which to use.'), + fix=(f'Check q-point {qpt_id} for duplication ' + 'or add splitting directions.'), ) raise ValueError(msg) @@ -425,9 +429,14 @@ def _read_castep_version(f: BinaryIO) -> Version: """Read next line and return CASTEP version; raise error if incompatible""" castep_version = Version(_read_entry(f).decode().strip()) if castep_version < Version('17.1'): - msg = dedent_and_fill(""" - Old castep file detected: Euphonic only supports post-Castep 17.1 - files. Please rerun the calculation with a newer version of Castep + msg = format_error( + 'Invalid castep version.', + reason=""" + Old castep file detected: + Euphonic only supports post-Castep 17.1 + files.""", + fix=""" + Rerun the calculation with a newer version of Castep with the original .cell file and a .castep file with a single line with the "continuation: " keyword and use the new output .castep_bin file in Euphonic.""") @@ -540,13 +549,13 @@ def read_interpolation_data( data_dict['sc_matrix'] = sc_matrix data_dict['cell_origins'] = cell_origins except NameError: - msg = dedent_and_fill(f""" - Force constants matrix could not be found in {filename}. - - - Ensure PHONON_WRITE_FORCE_CONSTANTS: true and a PHONON_FINE_METHOD - has been chosen when running CASTEP - """) + msg = format_error( + f'Invalid file ({filename}).', + reason='Force constants matrix could not be found', + fix=( + 'Ensure PHONON_WRITE_FORCE_CONSTANTS: true and ' + 'a PHONON_FINE_METHOD has been chosen when running CASTEP'), + ) raise RuntimeError(msg) from None # Set entries relating to dipoles @@ -675,7 +684,11 @@ def record_mark_read(file_obj): # Read 4 byte Fortran record marker rawdata = file_obj.read(4) if rawdata == b'': - msg = 'Problem reading binary file: unexpected EOF reached' + msg = format_error( + f'Issue reading binary file ({file_obj.name}).', + reason='Unexpected EOF reached.', + fix='Ensure file is valid.', + ) raise EOFError(msg) return struct.unpack('>i', rawdata)[0] @@ -699,8 +712,11 @@ def record_mark_read(file_obj): data = file_obj.read(begin) end = record_mark_read(file_obj) if begin != end: - msg = ('Problem reading binary file: beginning and end record markers ' - 'do not match') + msg = format_error( + f'Issue reading binary file ({file_obj.name}).', + reason='Beginning and end record markers do not match.', + fix='Ensure file is valid.', + ) raise OSError(msg) return data diff --git a/euphonic/readers/phonopy.py b/euphonic/readers/phonopy.py index 11c71a420..498047f6d 100644 --- a/euphonic/readers/phonopy.py +++ b/euphonic/readers/phonopy.py @@ -7,7 +7,7 @@ import numpy as np from euphonic.ureg import ureg -from euphonic.util import convert_fc_phases, dedent_and_fill +from euphonic.util import comma_join, convert_fc_phases, format_error HDF5_EXTS = {'hdf5', 'hd5', 'h5'} YAML_EXTS = {'yaml', 'yml', 'yl'} @@ -248,15 +248,16 @@ def read_phonon_data( phonon_dict = _extract_phonon_data_yaml( phonon_path, read_eigenvectors=read_eigenvectors) else: - msg = ( - f'File format {phonon_format} of {phonon_name} is not recognised' + msg = format_error( + f'Unrecognised format ({phonon_format}) for {phonon_name}.', + fix=f'Valid formats are: {comma_join((*HDF5_EXTS, *YAML_EXTS))}', ) raise ValueError(msg) if read_eigenvectors and 'eigenvectors' not in phonon_dict: - msg = ( - f"Eigenvectors couldn't be found in {phonon_path}, ensure " - f'--eigvecs was set when running Phonopy' + msg = format_error( + f"Eigenvectors couldn't be found in {phonon_path}.", + fix='Ensure --eigvecs is set when running Phonopy.', ) raise RuntimeError(msg) @@ -280,9 +281,12 @@ def read_phonon_data( umass = summary_dict['umass'] # Check phonon_file and summary_file are commensurate if 3*len(phonon_dict['atom_r']) != len(phonon_dict['frequencies'][0]): - msg = ( - f'Phonon file {phonon_path} not commensurate with summary ' - f'file {summary_path}. Please check contents' + msg = format_error( + "Phonopy files don't match.", + reason=( + f'Phonon file {phonon_path} not commensurate ' + f'with summary file {summary_path}.'), + fix='Ensure input files are consistent.', ) raise ValueError(msg) @@ -397,11 +401,14 @@ def _check_fc_shape(fc_shape: tuple[int, int], n_atoms: int, """ if (not ((fc_shape[0] == n_atoms or fc_shape[0] == n_cells*n_atoms) and fc_shape[1] == n_cells*n_atoms)): - msg = dedent_and_fill(f""" + msg = format_error( + 'Incompatible force constants.', + reason=f""" Force constants matrix with shape {fc_shape} read from {fc_filename} is not compatible with crystal read from {summary_filename} which has {n_atoms} atoms in the cell, and - {n_cells} cells in the supercell""") + {n_cells} cells in the supercell""", + fix='Ensure input files are consistent.') raise ValueError(msg) @@ -722,9 +729,11 @@ def read_interpolation_data( summary_dict['force_constants'] = _extract_force_constants_hdf5( fc_path, n_atoms, n_cells, summary_path) else: - msg = ( - f'Force constants file format "{fc_format}" of ' - f'"{fc_name}" is not recognised' + msg = format_error( + f'Unrecognised file format ({fc_format}) for {fc_name}.', + fix=('Provide fc_format excplicitly, as "phonopy" or "hdf5",' + 'or provide a file whose extension is one of ' + f'{comma_join(HDF5_EXTS)}.'), ) raise ValueError(msg) @@ -747,11 +756,14 @@ def read_interpolation_data( # 'dielectric' are written to phonopy.yaml, but if NAC = .FALSE. # 'nac_factor' will not be written. In this case raise error. if ('born' in summary_dict and 'nac_factor' not in summary_dict): - msg = dedent_and_fill(f""" + msg = format_error( + 'Unknown nac factor.', + reason=f""" nac_unit_conversion_factor could not be found in {summary_path} or the BORN file (if given), so units of the dielectric tensor cannot be determined. - """) + """, + fix='Run phonopy with NAC = .TRUE. .') raise KeyError(msg) # Units from summary_file diff --git a/euphonic/sampling.py b/euphonic/sampling.py index 1d3e1f085..5f0d52bca 100644 --- a/euphonic/sampling.py +++ b/euphonic/sampling.py @@ -9,7 +9,7 @@ import numpy as np from scipy.optimize import fmin -from euphonic.util import RNG, rng +from euphonic.util import RNG, format_error, rng _golden_ratio = (1 + np.sqrt(5)) / 2 @@ -304,7 +304,11 @@ def spherical_polar_improved(npts: int, If the number of points is not supported by this method """ if npts < 6: - msg = 'This sampling scheme has a minimum of 6 points' + msg = format_error( + 'Too few sampling points.', + reason='This sampling scheme has a minimum of 6 points.', + fix='Increase npts.', + ) raise ValueError(msg) # round from the solution of diff --git a/euphonic/spectra/base.py b/euphonic/spectra/base.py index cdb37d77f..3fb64da87 100644 --- a/euphonic/spectra/base.py +++ b/euphonic/spectra/base.py @@ -22,7 +22,6 @@ from euphonic.broadening import ( FWHM_TO_SIGMA, - ErrorFit, KernelShape, variable_width_broadening, ) @@ -34,7 +33,7 @@ ) from euphonic.readers.castep import read_phonon_dos_data from euphonic.ureg import ureg -from euphonic.util import dedent_and_fill, zips +from euphonic.util import comma_join, dedent_and_fill, format_error, zips from euphonic.validate import _check_constructor_inputs, _check_unit_conversion CallableQuantity = Callable[[Quantity], Quantity] @@ -113,23 +112,29 @@ def x_tick_labels(self) -> XTickLabels: @x_tick_labels.setter def x_tick_labels(self, value: XTickLabels) -> None: - err_msg = ( - 'x_tick_labels should be of type Sequence[Tuple[int, str]] e.g. ' - '[(0, "label1"), (5, "label2")]' + err_msg = format_error( + 'Invalid tick label type', + fix=( + 'x_tick_labels should be of type Sequence[Tuple[int, str]], ' + 'e.g. [(0, "label1"), (5, "label2")]'), ) + if value is not None: - if isinstance(value, Sequence): - for elem in value: - if not (isinstance(elem, tuple) - and len(elem) == 2 - and isinstance(elem[0], Integral) - and isinstance(elem[1], str)): - raise TypeError(err_msg) - # Ensure indices in x_tick_labels are plain ints as - # np.int64/32 etc. are not JSON serializable - value = [(int(idx), label) for idx, label in value] - else: + if not isinstance(value, Sequence): raise TypeError(err_msg) + + for elem in value: + match elem: + case (Integral(), str()): + pass + case _: + raise TypeError(err_msg) + + # Ensure indices in x_tick_labels are plain ints as + # np.int64/32 etc. are not JSON serializable + value = [(int(idx), label) for idx, label in value] + + self._x_tick_labels = value @abstractmethod @@ -239,7 +244,11 @@ def split(self: T, indices: Sequence[int] | np.ndarray = None, return self._split_by_tol(btol=btol) if btol is not None: - msg = 'Cannot set both indices and btol' + msg = format_error( + 'Invalid arguments.', + reason='Cannot set both indices and btol', + fix='Choose one of indices and btol.', + ) raise ValueError(msg) return self._split_by_indices(indices) @@ -263,24 +272,28 @@ def _has_bad_type(width: Any) -> bool: return width is not None and not isinstance(width, Real) if any(map(_has_bad_type, widths)): - msg = ('Inappropriate type found, widths for _broaden_data ' - 'must be Real (e.g. float) or None. Instead we have: [' - + ', '.join(type(width).__name__ for width in widths) - + '].') + msg = format_error( + 'Invalid widths for _broaden_data.', + reason=( + 'Widths must be Real or None, instead we have: [' + + comma_join(type(width).__name__ for width in widths) + + '].'), + fix='Ensure widths are valid.', + ) raise WidthTypeError(msg) shape_opts = ('gauss', 'lorentz') if shape not in shape_opts: - msg = ( - f'Invalid value for shape, got {shape}, ' - f'should be one of {shape_opts}' + msg = format_error( + f'Invalid value for shape ({shape}).', + fix=f'shape should be one of: {comma_join(shape_opts)}.', ) raise ValueError(msg) method_opts = ('convolve', None) if method not in method_opts: - msg = ( - f'Invalid value for method, got {method}, ' - f'should be one of {method_opts}' + msg = format_error( + f'Invalid value for method ({method}).', + fix=f'method should be one of {comma_join(method_opts)}.', ) raise ValueError(msg) @@ -300,9 +313,12 @@ def _has_bad_type(width: Any) -> bool: broadening by convolution may give incorrect results. """) if method is None: - raise ValueError( - msg + ' If you still want to broaden by convolution ' - 'please explicitly use the method="convolve" option.') + msg = format_error( + 'Unequal bin widths.', + reason=msg, + fix='Explicitly use the method="convolve" option.', + ) + raise ValueError(msg) warnings.warn(msg, stacklevel=3) @@ -315,7 +331,10 @@ def _has_bad_type(width: Any) -> bool: elif shape == 'lorentz': if width_convention != 'fwhm': - msg = 'Lorentzian function width must be specified as FWHM' + msg = format_error( + f'Invalid width convention ({width_convention}).', + fix='Lorentzian function width must be specified as FWHM.', + ) raise ValueError(msg) data_broadened = data for ax, (width, bin_data) in enumerate(zips(widths, bin_centres)): @@ -360,7 +379,10 @@ def _gaussian_width_to_bin_sigma( case 'std': sigma = width case _: - msg = "Width convention must be 'std' or 'fwhm'" + msg = format_error( + 'Invalid width convention.', + fix="Width convention must be 'std' or 'fwhm'", + ) raise ValueError(msg) mean_bin_size = np.mean(np.diff(ax_bin_centres)) @@ -397,9 +419,10 @@ def _is_bin_edge(data_length: int, bin_length: int) -> bool: return True if bin_length == data_length: return False - msg = ( + msg = format_error( f'Unexpected data axis length {data_length} ' - f'for bin axis length {bin_length}' + f'for bin axis length {bin_length}', + fix=f'bin_lengths must be {data_length} or {data_length + 1}', ) raise ValueError(msg) @@ -460,6 +483,7 @@ def get_bin_widths(self, *, restrict_range: bool = True) -> Quantity: return np.diff(self.get_bin_edges(restrict_range=restrict_range)) def assert_regular_bins(self, + *, message: str = '', rtol: float = 1e-5, atol: float = 0., @@ -474,7 +498,7 @@ def assert_regular_bins(self, Parameters ---------- message - Text appended to ValueError for more informative output. + Text included in ValueError for more informative output. rtol Relative tolerance for 'close enough' values @@ -496,8 +520,11 @@ def assert_regular_bins(self, # Need to cast to magnitude to use isclose() with atol before Pint 0.21 if not np.all(np.isclose(bin_widths.magnitude, bin_widths.magnitude[0], rtol=rtol, atol=atol)): - raise AssertionError('Not all x-axis bins are the same width. ' - + message) + msg = format_error( + 'Invalid bin widths.', + reason='Not all x-axis bins are the same width. ' + message, + fix='Ensure x_data has regular spacing.') + raise AssertionError(msg) class Spectrum1D(Spectrum): @@ -709,7 +736,6 @@ def broaden(self: T, x_width: CallableQuantity, width_lower_limit: Quantity | None = None, width_convention: Literal['fwhm', 'std'] = 'fwhm', width_interpolation_error: float = 0.01, - width_fit: ErrorFit = 'cheby-log', ) -> T: ... def broaden(self: T, x_width, @@ -718,7 +744,6 @@ def broaden(self: T, x_width, width_lower_limit=None, width_convention='fwhm', width_interpolation_error=0.01, - width_fit='cheby-log', ) -> T: """ Broaden y_data and return a new broadened spectrum object @@ -748,10 +773,6 @@ def broaden(self: T, x_width, When x_width is a callable function, variable-width broadening is implemented by an approximate kernel-interpolation scheme. This parameter determines the target error of the kernel approximations. - width_fit - Select parametrisation of kernel width spacing to - width_interpolation_error. 'cheby-log' is recommended: for shape - 'gauss', 'cubic' is also available. Returns ------- @@ -790,7 +811,6 @@ def broaden(self: T, x_width, width_convention=width_convention, adaptive_error=width_interpolation_error, shape=shape, - fit=width_fit, ) else: msg = 'x_width must be a Quantity or Callable' @@ -914,7 +934,6 @@ def broaden(self: T, y_width_lower_limit: Quantity = None, width_convention: Literal['fwhm', 'std'] = 'fwhm', width_interpolation_error: float = 0.01, - width_fit: ErrorFit = 'cheby-log', ) -> T: """ Broaden z_data and return a new broadened Spectrum2D object @@ -954,10 +973,6 @@ def broaden(self: T, When x_width is a callable function, variable-width broadening is implemented by an approximate kernel-interpolation scheme. This parameter determines the target error of the kernel approximations. - width_fit - Select parametrisation of kernel width spacing to - width_interpolation_error. 'cheby-log' is recommended: for shape - 'gauss', 'cubic' is also available. Returns ------- @@ -1018,7 +1033,7 @@ def broaden(self: T, width_convention=width_convention, width_interpolation_error=width_interpolation_error, shape=shape, - width_fit=width_fit) + ) return spectrum @@ -1031,7 +1046,6 @@ def _broaden_spectrum2d_with_function( width_convention: Literal['fwhm', 'std'] = 'fwhm', width_interpolation_error: float = 1e-2, shape: KernelShape = 'gauss', - width_fit: ErrorFit = 'cheby-log', ) -> 'Spectrum2D': """ Apply value-dependent Gaussian broadening to one axis of Spectrum2D @@ -1065,7 +1079,7 @@ def _broaden_spectrum2d_with_function( width_convention=width_convention, adaptive_error=width_interpolation_error, shape=shape, - fit=width_fit) + ) if axis == 'x': z_broadened = z_broadened.T @@ -1172,19 +1186,16 @@ def get_bin_widths( bins = self.get_bin_edges(bin_ax, restrict_range=restrict_range) return np.diff(bins) - def assert_regular_bins( # pylint: disable=arguments-renamed + def assert_regular_bins( self, - bin_ax: Literal['x', 'y'], + *, + bin_ax: Literal['x', 'y'] = 'y', message: str = '', rtol: float = 1e-5, atol: float = 0., restrict_range: bool = True, ) -> None: - """Raise AssertionError if x-axis bins are not evenly spaced. - - Note that the positional arguments are different from - Spectrum1D.assert_regular_bins: it is strongly recommended to only use - keyword arguments with this method. + """Raise AssertionError if bins are not evenly spaced. Parameters ---------- @@ -1212,8 +1223,12 @@ def assert_regular_bins( # pylint: disable=arguments-renamed bin_widths = self.get_bin_widths(bin_ax, restrict_range=restrict_range) if not np.all(np.isclose(bin_widths, bin_widths[0], rtol=rtol, atol=atol)): - raise AssertionError( - f'Not all {bin_ax}-axis bins are the same width. ' + message) + msg = format_error( + 'Invalid bin widths.', + reason=(f'Not all {bin_ax}-axis bins are the same width. ' + + message), + fix=f'Ensure {bin_ax}_data has regular spacing.') + raise AssertionError(msg) def to_dict(self) -> dict[str, Any]: """ @@ -1295,22 +1310,31 @@ def apply_kinematic_constraints(spectrum: Spectrum2D, try: (1 * spectrum.x_data.units).to('1/angstrom') except DimensionalityError as error: + msg = format_error( + f'Invalid x_data units ({spectrum.x_data.units}).', + fix='x_data needs to have wavevector units (i.e. 1/length)', + ) msg = 'x_data needs to have wavevector units (i.e. 1/length)' raise ValueError(msg) from error try: (1 * spectrum.y_data.units).to('eV', 'spectroscopy') except DimensionalityError as error: - msg = 'y_data needs to have energy (or wavenumber) units' + msg = format_error( + f'Invalid y_data units ({spectrum.y_data.units}).', + fix='y_data needs to have energy (or wavenumber) units', + ) raise ValueError(msg) from error momentum2_to_energy = 0.5 * (ureg('hbar^2 / neutron_mass') .to('meV angstrom^2')) if (e_i is None) == (e_f is None): - msg = dedent_and_fill(""" - Exactly one of e_i and e_f should be set. - (The other value will be derived from energy transfer). - """) + msg = format_error( + 'Invalid arguments.', + fix=( + 'Exactly one of e_i and e_f should be set. ' + '(The other value will be derived from energy transfer).'), + ) raise ValueError(msg) if e_i is None: @@ -1382,7 +1406,10 @@ def _distribution_1d(xbins: np.ndarray, ) -> np.ndarray: x = _get_dist_bins(xbins) if shape != 'lorentz': - msg = "Expected shape: 'lorentz'" + msg = format_error( + f'Invalid shape ({shape}).', + fix='Shape must be "lorentz"', + ) raise ValueError(msg) dist = _lorentzian(x, xwidth) diff --git a/euphonic/spectra/collections.py b/euphonic/spectra/collections.py index 6ca9fb253..9d49c4620 100644 --- a/euphonic/spectra/collections.py +++ b/euphonic/spectra/collections.py @@ -24,11 +24,11 @@ from toolz.itertoolz import groupby, pluck from typing_extensions import Self -from euphonic.broadening import ErrorFit, KernelShape +from euphonic.broadening import KernelShape from euphonic.io import _obj_to_dict, _process_dict from euphonic.readers.castep import read_phonon_dos_data from euphonic.ureg import ureg -from euphonic.util import dedent_and_fill +from euphonic.util import format_error from euphonic.validate import _check_constructor_inputs from euphonic.version import __version__ @@ -231,18 +231,13 @@ def _validate_item( of float or bool was provided when ints are needed. """ - if isinstance(item, Integral): - return - if isinstance(item, slice): - if (item.stop is not None) and (item.stop >= len(self)): - msg = f'index "{item.stop}" out of range' - raise IndexError(msg) + if isinstance(item, (Integral, slice)): return if not all(isinstance(i, Integral) for i in item): - msg = ( - f'Index "{item}" should be an integer, slice ' - f'or sequence of ints' + msg = format_error( + f'Invalid type ({item}).', + fix='Index must be an integer, slice or sequence of ints.', ) raise TypeError(msg) @@ -373,9 +368,13 @@ def ensure_sequence(value: int | str | Sequence[int | str], selected_indices.extend(self._select_indices(**selection)) if not selected_indices: - msg = ( - f'No spectra found with matching metadata ' - f'for {select_key_values}' + msg = format_error( + 'Metadata not found.', + reason=( + f'No spectra found with matching metadata ' + f'for {select_key_values}.' + ), + fix='Try different selection keys or ensure metadata exist.', ) raise ValueError(msg) @@ -434,11 +433,14 @@ def _check_metadata(self) -> None: n_lines = len(self.metadata['line_data']) if n_lines != collection_size: - msg = dedent_and_fill(f""" + msg = format_error( + 'Data mismatch.', + reason=f""" {self._spectrum_data_name()} contains {collection_size} spectra, but metadata["line_data"] contains {n_lines} - entries - """) + entries. + """, + fix='Ensure metadata["line_data"] has correct length.') raise ValueError(msg) def group_by(self, *line_data_keys: str) -> Self: @@ -515,7 +517,10 @@ def from_dict(cls: Self, d: dict) -> Self: @classmethod def _item_type_check(cls, spectrum) -> None: if not isinstance(spectrum, cls._item_type): - msg = f'Item is not of type {cls._item_type.__name__}.' + msg = format_error( + f'Invalid type ({type(spectrum).__name__}).', + fix='Item must be of type {cls._item_type.__name__}.', + ) raise TypeError(msg) @@ -624,13 +629,26 @@ def _from_spectra_data_check(spectrum, x_data, y_data_units, x_tick_labels, ) -> None: if (spectrum.y_data.units != y_data_units or spectrum.x_data.units != x_data.units): - msg = 'Spectrum units in sequence are inconsistent' + msg = format_error( + 'Inconsistent units.', + reason=f""" + x_data units ({spectrum.x_data.units} & {x_data.units}) or + y_data units ({spectrum.y_data.units} & {y_data_units}) + don't match. + """, + fix='Ensure units match') raise ValueError(msg) if not np.allclose(spectrum.x_data, x_data): - msg = 'x_data in sequence are inconsistent' + msg = format_error( + 'Inconsistent x_data', + fix='Spectrum x_data must equal x_data', + ) raise ValueError(msg) if spectrum.x_tick_labels != x_tick_labels: - msg = 'x_tick_labels in sequence are inconsistent' + msg = format_error( + 'Inconsistent x_tick_labels', + fix='Ensure tick labels match', + ) raise ValueError(msg) @classmethod @@ -741,7 +759,6 @@ def broaden(self: T, x_width: CallableQuantity, width_lower_limit: Quantity | None = None, width_convention: Literal['fwhm', 'std'] = 'fwhm', width_interpolation_error: float = 0.01, - width_fit: ErrorFit = 'cheby-log', ) -> T: ... def broaden(self: T, @@ -751,7 +768,6 @@ def broaden(self: T, width_lower_limit=None, width_convention='fwhm', width_interpolation_error=0.01, - width_fit='cheby-log', ) -> T: """ Individually broaden each line in y_data, returning a new @@ -782,10 +798,6 @@ def broaden(self: T, When x_width is a callable function, variable-width broadening is implemented by an approximate kernel-interpolation scheme. This parameter determines the target error of the kernel approximations. - width_fit - Select parametrisation of kernel width spacing to - width_interpolation_error. 'cheby-log' is recommended: for shape - 'gauss', 'cubic' is also available. Returns ------- @@ -822,10 +834,13 @@ def broaden(self: T, width_lower_limit=width_lower_limit, width_convention=width_convention, width_interpolation_error=width_interpolation_error, - width_fit=width_fit) + ) for spectrum in self]) - msg = 'x_width must be a Quantity or Callable' + msg = format_error( + f'Invalid type ({type(x_width).__name__}).', + fix='x_width must be a Quantity or Callable', + ) raise TypeError(msg) @classmethod @@ -956,10 +971,20 @@ def _from_spectra_data_check( ) -> None: """Check spectrum data units and x_tick_labels are consistent""" if spectrum_0_data_units != spectrum_i_data_units: - msg = 'Spectrum units in sequence are inconsistent' + msg = format_error( + 'Inconsistent units.', + reason='Spectrum units in sequence are inconsistent.', + fix=('Ensure Spectrum units are consistent ' + 'for all sequence items.'), + ) raise ValueError(msg) if x_tick_labels != spectrum_i_x_tick_labels: - msg = 'x_tick_labels in sequence are inconsistent' + msg = format_error( + 'Inconsistent x_tick_labels.', + reason='x_tick_labels units in sequence are inconsistent.', + fix=('Ensure x_tick_labels are consistent ' + 'for all sequence items.'), + ) raise ValueError(msg) @staticmethod @@ -975,10 +1000,20 @@ def _from_spectra_bins_check(ref_bins_data: dict[str, Quantity], for key, ref_bins in ref_bins_data.items(): item_bins = getattr(spectrum, key) if not np.allclose(item_bins.magnitude, ref_bins.magnitude): - msg = 'Bins in sequence are inconsistent' + msg = format_error( + 'Inconsistent bins.', + reason='Bins in sequence are inconsistent.', + fix=('Ensure bins are consistent ' + 'for all sequence items.'), + ) raise ValueError(msg) if item_bins.units != ref_bins.units: - msg = 'Bin units in sequence are inconsistent' + msg = format_error( + 'Inconsistent bin units.', + reason='Bins units in sequence are inconsistent.', + fix=('Ensure bins units are consistent ' + 'for all sequence items.'), + ) raise ValueError(msg) @classmethod @@ -993,7 +1028,10 @@ def from_spectra( """ if len(spectra) < 1: - msg = 'At least one spectrum is needed for collection' + msg = format_error( + 'No spectra provided.', + fix='Must have at least one spectrum for collection.', + ) raise IndexError(msg) cls._item_type_check(spectra[0]) diff --git a/euphonic/structure_factor.py b/euphonic/structure_factor.py index b726a61ef..f5b2de91d 100644 --- a/euphonic/structure_factor.py +++ b/euphonic/structure_factor.py @@ -8,7 +8,7 @@ from euphonic.qpoint_frequencies import QpointFrequencies from euphonic.spectra import Spectrum1D, Spectrum2D from euphonic.ureg import Quantity, ureg -from euphonic.util import dedent_and_fill +from euphonic.util import format_error from euphonic.validate import _check_constructor_inputs, _check_unit_conversion @@ -322,17 +322,26 @@ def _bose_factor(self, temperature: Quantity | None = None, if self.temperature is not None: if (temperature is not None and not np.isclose(temperature, self.temperature)): - msg = dedent_and_fill(f""" + msg = format_error( + 'Inconsistent temperature.', + reason=f""" Temperature provided to calculate the Bose factor ({temperature:~P}) is not consistent with the temperature - stored in StructureFactor ({self.temperature:~P})""") + stored in StructureFactor ({self.temperature:~P}).""", + fix=('Do not provide temperature, or ensure ' + 'temperatures are consistent.')) raise ValueError(msg) temperature = self.temperature if temperature is None: - msg = dedent_and_fill(""" + msg = format_error( + 'No temperature provided.', + reason=""" When calculating the Bose factor, no temperature was - provided, and no temperature could be found in StructureFactor - """) + provided, and no temperature could be found in StructureFactor. + """, + fix=('Ensure temperature is ' + 'provided as argument or in StructureFactor.'), + ) raise NoTemperatureError(msg) k_B = (1*ureg.k).to('E_h/K').magnitude temp = temperature.to('K').magnitude diff --git a/euphonic/util.py b/euphonic/util.py index bd9b429f4..dd06efafb 100644 --- a/euphonic/util.py +++ b/euphonic/util.py @@ -1,4 +1,4 @@ -from collections.abc import Sequence +from collections.abc import Iterable, Sequence from contextlib import contextmanager from functools import partial, reduce from importlib.resources import files @@ -9,6 +9,7 @@ from pathlib import Path import sys import textwrap +from typing import Any import warnings import numpy as np @@ -28,6 +29,21 @@ zips = partial(zip, strict=True) +def comma_join(x: Iterable[Any], /): + """Shorthand to join objects as strings with commas. + + Parameters + ---------- + x : Sequence[Any] + Entities to join. + + Examples + -------- + >>> from euphonic.util import comma_join + >>> comma_join((None, "a", 1)) + 'None, a, 1' + """ + return ', '.join(map(str, x)) def dedent_and_fill(text: str) -> str: r"""Clean up indented multiline string for display to user. @@ -54,6 +70,27 @@ def dedent_and_fill(text: str) -> str: return '\n\n'.join(textwrap.fill(paragraph) for paragraph in paragraphs) +def format_error(summary: str, *, reason: str = '', fix: str) -> str: + """Format errors to standard form. + + Parameters + ---------- + summary : str + Summary of what went wrong, should not exceed 80 characters. + reason : str, optional + Extended summary detailing the cause of the issue. + fix : str + Suggested remedy for issue. + + Returns + ------- + str + Formatted error message. + """ + return '\n\n'.join( + map(dedent_and_fill, + filter(None, (summary, reason, fix)))) + def direction_changed(qpts: np.ndarray, tolerance: float = 5e-6, ) -> np.ndarray: """ @@ -263,40 +300,43 @@ def custom_decode(dct): with open(filename) as fd: file_data = json.load(fd, object_hook=custom_decode) else: - msg = dedent_and_fill(f""" - No data files known for collection "{collection}". - Available collections: {', '.join(_reference_data_files)} - """, + msg = format_error( + f'No data files known for collection "{collection}".', + fix=f'Available collections: {comma_join(_reference_data_files)}.', ) raise ValueError(msg) if 'physical_property' not in file_data: - msg = 'Data file does not contain required key "physical_property".' + msg = format_error( + 'Data file does not contain required key "physical_property".', + fix='Ensure file is formatted correctly.', + ) raise AttributeError(msg) data = file_data['physical_property'].get(physical_property) if data is None: - msg = dedent_and_fill(f""" - No such collection "{collection}" with property - "{physical_property}". Available properties for this collection: - """ + ', '.join(file_data['physical_property']), + msg = format_error( + (f'No such collection "{collection}" ' + f'with property "{physical_property}".'), + fix=('Available properties for this collection:' + f'{comma_join(file_data["physical_property"])}.'), ) raise ValueError(msg) unit_str = data.get('__units__') if unit_str is None: - msg = ( - f'Reference data file "{filename}" does not ' - 'specify dimensions with "__units__" metadata.' + msg = format_error( + f'No units in file ({filename}).', + fix='Ensure file specifies dimensions with "__units__" metadata.', ) raise ValueError(msg) try: unit = ureg(unit_str) except UndefinedUnitError as exc: - msg = ( - f'Units "{unit_str}" from data file "{filename}" ' - 'are not supported by the Euphonic unit register.' + msg = format_error( + f'Unsupported units ({unit_str}) from data file "{filename}".', + fix='Ensure units are supported by Euphonic unit register.', ) raise ValueError(msg) from exc @@ -331,9 +371,9 @@ def mode_gradients_to_widths(mode_gradients: Quantity, cell_vectors: Quantity, if modg.ndim == 3 and modg.shape[2] == 3: modg = np.linalg.norm(modg, axis=2) elif modg.ndim != 2: - msg = ( - f'Unexpected shape for mode_gradients {modg.shape}, ' - f'expected (n_qpts, n_modes) or (n_qpts, n_modes, 3)' + msg = format_error( + f'Unexpected shape for mode_gradients ({modg.shape}).', + fix='Shape should be (n_qpts, n_modes) or (n_qpts, n_modes, 3).', ) raise ValueError(msg) @@ -405,12 +445,12 @@ def convert_fc_phases(force_constants: np.ndarray, atom_r: np.ndarray, n_atoms_uc = len(uc_to_sc_atom_idx) n_cells = int(np.rint(np.absolute(np.linalg.det(sc_matrix)))) if n_atoms_sc/n_atoms_uc - n_cells != 0: - msg = dedent_and_fill(f""" - Inconsistent numbers of cells in the supercell, unit cell has - {n_atoms_uc} atoms, and supercell has {n_atoms_sc} atoms, but - sc_matrix determinant suggests there should be {n_cells} cells in - the supercell - """, + msg = format_error( + 'Inconsistent numbers of cells in the supercell.', + reason=(f'Unit cell has {n_atoms_uc} atoms, ' + f'and supercell has {n_atoms_sc} atoms'), + fix=('sc_matrix determinant suggests there should ' + f'be {n_cells} cells in the supercell'), ) raise ValueError(msg) # Get cell origins for all atoms @@ -419,10 +459,10 @@ def convert_fc_phases(force_constants: np.ndarray, atom_r: np.ndarray, np.abs(cell_origins_per_atom - np.rint(cell_origins_per_atom)) > cell_origins_tol)[0] if len(non_int) > 0: - msg = dedent_and_fill(f""" - Non-integer cell origins for atom(s) - {", ".join(np.unique(non_int).astype(str))}, - check coordinates and indices are correct""", + msg = format_error( + ('Non-integer cell origins for atom(s) ' + f'({comma_join(np.unique(non_int).astype(str))}).'), + fix='Check coordinates and indices are correct.', ) raise RuntimeError(msg) cell_origins_per_atom = np.rint(cell_origins_per_atom).astype(np.int32) @@ -458,9 +498,11 @@ def convert_fc_phases(force_constants: np.ndarray, atom_r: np.ndarray, co_idx = j break else: - msg = dedent_and_fill(""" - Couldn't determine cell origins for force constants matrix - """) + msg = format_error( + ("Couldn't determine cell origins " + "for force constants matrix."), + fix='Ensure supercell origins are valid.', + ) raise ValueError(msg) cell_origins_map[i] = co_idx @@ -806,7 +848,10 @@ def _get_supercell_relative_idx(cell_origins: np.ndarray, if np.all(dist_min <= 16*sys.float_info.epsilon): break if np.any(dist_min > 16*sys.float_info.epsilon): - msg = "Couldn't find supercell relative index" + msg = format_error( + "Couldn't find supercell relative index.", + fix='Supercell may not be equivalent.', + ) raise ValueError(msg) return sc_relative_index diff --git a/euphonic/validate.py b/euphonic/validate.py index 381bdf025..54ceb8211 100644 --- a/euphonic/validate.py +++ b/euphonic/validate.py @@ -5,6 +5,7 @@ from pint import DimensionalityError from euphonic.ureg import ureg +from euphonic.util import comma_join, format_error def _check_constructor_inputs( @@ -45,18 +46,20 @@ def _check_constructor_inputs( if not isinstance(typ, list): typ = [typ] # noqa: PLW2901 redefined-loop-name if not any(isinstance(obj, t) for t in typ): - msg = ( - f"The type of {name} {type(obj)} doesn't " - f'match the expected type(s) {typ}' + msg = format_error( + f'Invalid type for {name} ({type(obj).__name__}).', + fix=(f'Ensure {name} is cast to a correct type. ' + 'Valid types are: ' + f'{comma_join(t.__name__ for t in typ)}.'), ) raise TypeError(msg) if hasattr(obj, 'shape') and shape: if not isinstance(shape, list): shape = [shape] # noqa: PLW2901 (redefined-loop-name) if not any(obj.shape == _replace_dim(s, obj.shape) for s in shape): - msg = ( - f"The shape of {name} {obj.shape} doesn't match " - f'the expected shape(s) {shape}' + msg = format_error( + f'Invalid shape for {name} ({obj.shape}).', + fix=f'Expected shape is {shape}', ) raise ValueError(msg) @@ -90,9 +93,9 @@ def _check_unit_conversion(obj: object, attr_name: str, attr_value: Any, ureg(getattr(obj, attr_name)).ito(attr_value, 'reciprocal_spectroscopy') except DimensionalityError as err: - msg = ( - f'"{attr_value}" is not a known dimensionally-consistent ' - f'unit for "{attr_name}"' + msg = format_error( + f'Unknown unit type for {attr_name} ({attr_value}).', + fix='Check unit dimensions are valid.', ) raise ValueError(msg) from err diff --git a/tests_and_analysis/test/euphonic_test/test_broadening.py b/tests_and_analysis/test/euphonic_test/test_broadening.py index fac19a3da..31cf8e28b 100644 --- a/tests_and_analysis/test/euphonic_test/test_broadening.py +++ b/tests_and_analysis/test/euphonic_test/test_broadening.py @@ -67,20 +67,19 @@ def width_function(x): width_convention='STD', weights=(example.y * example.bin_width).to('dimensionless').magnitude, # Convert from spectrum heights to counts adaptive_error=2e-4, - fit='cheby-log') + ) npt.assert_allclose(exact, poly_broadened.to('1/meV').magnitude, atol=1e-4) @pytest.mark.parametrize( - 'width_convention, fit, adaptive_error, message', - [('STD', 'cheby-log', 0.001, 'Standard deviation unavailable'), - ('UNIMPLEMENTED', 'cheby-log', 0.001, 'must be "std" or "fwhm"'), - ('FWHM', 'UNIMPLEMENTED', 0.001, 'Fit "UNIMPLEMENTED" is not available'), - ('FWHM', 'cheby-log', 0.00001, 'Target error is out of fit range'), + 'width_convention, adaptive_error, message', + [('STD', 0.001, 'Standard deviation unavailable'), + ('UNIMPLEMENTED', 0.001, 'must be "std" or "fwhm"'), + ('FWHM', 0.00001, r'Target error \([^)]+\) is out of fit range'), ]) -def test_variable_width_broadening_errors(example, width_convention, fit, adaptive_error, message): +def test_variable_width_broadening_errors(example, width_convention, adaptive_error, message): with pytest.raises(ValueError, match=message): variable_width_broadening( @@ -90,7 +89,6 @@ def test_variable_width_broadening_errors(example, width_convention, fit, adapti width_convention=width_convention, weights=example.y.magnitude, shape='lorentz', - fit=fit, adaptive_error=adaptive_error) @@ -111,11 +109,11 @@ def test_area_unchanged_for_broadened_dos(material, qpt_freqs_json, weights = np.full(qpt_freqs.frequencies.shape, 1/(qpt_freqs.n_qpts*qpt_freqs.crystal.n_atoms)) variable_width_broaden = width_interpolated_broadening( - ebins, - qpt_freqs.frequencies, - mode_widths, weights, - 0.01, - fit='cheby-log') + ebins, + qpt_freqs.frequencies, + mode_widths, weights, + 0.01, + ) ebins_centres = ebins.magnitude[:-1] + 0.5*np.diff(ebins.magnitude) assert dos.y_data.units == 1/ebins.units dos_area = simpson(dos.y_data.magnitude, x=ebins_centres) @@ -126,12 +124,14 @@ def test_area_unchanged_for_broadened_dos(material, qpt_freqs_json, @pytest.mark.parametrize( - ('material, qpt_freqs_json, mode_widths_json,' - 'expected_dos_json, ebins'), [ - ('quartz', 'toy_quartz_qpoint_frequencies.json', - 'toy_quartz_mode_widths.json', - 'toy_quartz_adaptive_dos.json', - np.arange(0, 40, 0.1)*ureg('meV'))]) + ('material, qpt_freqs_json, mode_widths_json, expected_dos_json, ebins'), + [('quartz', + 'toy_quartz_qpoint_frequencies.json', + 'toy_quartz_mode_widths.json', + 'toy_quartz_adaptive_dos.json', + np.arange(0, 40, 0.1)*ureg('meV')), + ], +) def test_lower_bound_widths_broadened(material, qpt_freqs_json, mode_widths_json, expected_dos_json, ebins): @@ -143,9 +143,8 @@ def test_lower_bound_widths_broadened(material, qpt_freqs_json, mode_widths = get_mode_widths(get_fc_path(mode_widths_json)) weights = np.ones(qpt_freqs.frequencies.shape) * \ np.full(qpt_freqs.n_qpts, 1/qpt_freqs.n_qpts)[:, np.newaxis] - dos = width_interpolated_broadening(ebins, qpt_freqs.frequencies, - mode_widths, weights, 0.01, - fit='cubic') + dos = width_interpolated_broadening( + ebins, qpt_freqs.frequencies, mode_widths, weights, 0.01) expected_dos = get_expected_spectrum1d(expected_dos_json) npt.assert_allclose(expected_dos.y_data.magnitude, dos.magnitude) @@ -160,9 +159,8 @@ def test_uneven_bin_width_raises_warning(): ebins = np.concatenate((np.arange(0, 100, 0.1), np.arange(100, 155, 0.2)))*ureg('meV') with pytest.warns(UserWarning): - width_interpolated_broadening(ebins, qpt_freqs.frequencies, - mode_widths, weights, 0.01, - fit='cubic') + width_interpolated_broadening( + ebins, qpt_freqs.frequencies, mode_widths, weights, 0.01) @pytest.mark.parametrize( diff --git a/tests_and_analysis/test/euphonic_test/test_cli_utils.py b/tests_and_analysis/test/euphonic_test/test_cli_utils.py index 3adcbe59b..2811e0cd7 100644 --- a/tests_and_analysis/test/euphonic_test/test_cli_utils.py +++ b/tests_and_analysis/test/euphonic_test/test_cli_utils.py @@ -17,19 +17,19 @@ def test_get_cli_parser_error(): with pytest.raises( - ValueError, match='No band-data-only tools have been defined.'): + ValueError, match=r'No band-data-only tools have been defined\.'): _get_cli_parser(features={'read-modes'}) with pytest.raises( ValueError, - match='"adaptive-broadening" cannot be applied without "ebins"'): + match='"adaptive-broadening" cannot be used without "ebins"'): _get_cli_parser(features={'adaptive-broadening'}) def test_get_energy_bins_error(): with pytest.raises( ValueError, - match='Maximum energy should be greater than minimum.'): + match=r'Maximum energy should be greater than minimum\.'): _get_energy_bins(modes=None, n_ebins=0, emin=1, emax=0) @@ -48,13 +48,13 @@ def test_load_phonopy_errors(): 'phonopy_files', 'CaHgO2', 'full_force_constants.hdf5') with pytest.raises( - ValueError, match='must be accompanied by phonopy.yaml'): + ValueError, match=r'Missing phonopy\.yaml\.'): _load_phonopy_file(fc_file) def test_load_data_extension_error(): with pytest.raises( - ValueError, match='File format was not recognised.'): + ValueError, match=r'Ensure file format in known formats\.'): load_data_from_file('nonexistent.wrong_suffix') @@ -88,4 +88,3 @@ def test_find_force_constants(mocked_fc_from_phonopy): 'summary_name': 'phonopy_nofc.yaml', 'fc_name': 'force_constants.hdf5', } - diff --git a/tests_and_analysis/test/euphonic_test/test_crystal.py b/tests_and_analysis/test/euphonic_test/test_crystal.py index f158d3db3..001b42ec8 100644 --- a/tests_and_analysis/test/euphonic_test/test_crystal.py +++ b/tests_and_analysis/test/euphonic_test/test_crystal.py @@ -474,5 +474,5 @@ def test_get_symmetry_equivalent_atoms_errors(self, mocker): return_value=None) crystal = get_crystal('LZO') with pytest.raises(RuntimeError, - match='spglib.get_symmetry returned None'): + match=r'spglib\.get_symmetry unable to determine symmetry\.'): crystal.get_symmetry_equivalent_atoms() diff --git a/tests_and_analysis/test/euphonic_test/test_plot.py b/tests_and_analysis/test/euphonic_test/test_plot.py index 646b4382d..f0a0b9a3b 100644 --- a/tests_and_analysis/test/euphonic_test/test_plot.py +++ b/tests_and_analysis/test/euphonic_test/test_plot.py @@ -51,7 +51,7 @@ def mocked_import(name, *args, **kwargs): with pytest.raises(ModuleNotFoundError) as mnf_error: reload(euphonic.plot) - assert ('Cannot import Matplotlib for plotting' + assert ('Cannot import matplotlib for plotting' in mnf_error.value.args[0]) diff --git a/tests_and_analysis/test/euphonic_test/test_spectrum1d.py b/tests_and_analysis/test/euphonic_test/test_spectrum1d.py index c2dbeb53f..080453d62 100644 --- a/tests_and_analysis/test/euphonic_test/test_spectrum1d.py +++ b/tests_and_analysis/test/euphonic_test/test_spectrum1d.py @@ -602,7 +602,7 @@ def test_copy(self): class TestBasePrivateFunctions: def test_distribution_1d_error(self): - with pytest.raises(ValueError, match="Expected shape: 'lorentz'"): + with pytest.raises(ValueError, match='Shape must be "lorentz"'): _distribution_1d(xbins=np.array([0., 1., 2.]), xwidth=4., shape='non-implemented-shape') diff --git a/tests_and_analysis/test/euphonic_test/test_spectrum1dcollection.py b/tests_and_analysis/test/euphonic_test/test_spectrum1dcollection.py index 290af4472..3ff58d822 100644 --- a/tests_and_analysis/test/euphonic_test/test_spectrum1dcollection.py +++ b/tests_and_analysis/test/euphonic_test/test_spectrum1dcollection.py @@ -472,8 +472,7 @@ def test_index_sequence(self, spectrum, index, expected_spectrum): np.arange(4.0), TypeError), (get_spectrum1dcollection('gan_bands.json'), 6, IndexError), - (get_spectrum1dcollection('gan_bands.json'), - slice(2, 6), IndexError)]) + ]) def test_index_errors(self, spectrum, index, expected_error): with pytest.raises(expected_error): spectrum[index] diff --git a/tests_and_analysis/test/euphonic_test/test_spectrum2d.py b/tests_and_analysis/test/euphonic_test/test_spectrum2d.py index 8da95b989..f8e24e47f 100644 --- a/tests_and_analysis/test/euphonic_test/test_spectrum2d.py +++ b/tests_and_analysis/test/euphonic_test/test_spectrum2d.py @@ -361,16 +361,7 @@ def test_split(self, args, spectrum2d_file, split_spectrum_files): 'y_width': (lambda y: np.polyval([0., -0.4, 3.], y.to('J').magnitude, ) * ureg('J')), - 'width_fit': 'cubic'}, - 'synthetic_x.json', 'synthetic_x_poly_broadened.json', - does_not_raise())), - (({'x_width': (lambda x: np.polyval([0.2, -0.5], - x.to('1/nm').magnitude, - ) * ureg('1/nm')), - 'y_width': (lambda y: np.polyval([0., -0.4, 3.], - y.to('J').magnitude, - ) * ureg('J')), - 'width_fit': 'cheby-log'}, + }, 'synthetic_x.json', 'synthetic_x_poly_broadened_cheby.json', does_not_raise())), ]) diff --git a/tests_and_analysis/test/script_tests/test_intensity_map.py b/tests_and_analysis/test/script_tests/test_intensity_map.py index 360f91b24..351d4cc83 100644 --- a/tests_and_analysis/test/script_tests/test_intensity_map.py +++ b/tests_and_analysis/test/script_tests/test_intensity_map.py @@ -139,7 +139,8 @@ def test_qpoint_frequencies_incompatible_args_raises_type_error( '--temperature', '300']]) def test_qpoint_modes_debyewaller_raises_type_error( self, intensity_map_args): - with pytest.raises(TypeError, match='Force constants data'): + with pytest.raises(TypeError, + match='Force constants required to generate the Debye-Waller factor'): euphonic.cli.intensity_map.main(intensity_map_args) @pytest.mark.parametrize('intensity_map_args', [