From 15dfa87b21acfe89d4bd7e8618ea337a315613f7 Mon Sep 17 00:00:00 2001 From: ElliottKasoar <45317199+ElliottKasoar@users.noreply.github.com> Date: Fri, 8 Nov 2024 12:05:09 +0000 Subject: [PATCH] Apply suggestions from code review Co-authored-by: Jacob Wilkins <46597752+oerc0122@users.noreply.github.com> --- janus_core/calculations/phonons.py | 20 ++++++++++---------- janus_core/cli/phonons.py | 10 +++++----- tests/test_phonons_cli.py | 21 ++++++++++----------- 3 files changed, 25 insertions(+), 26 deletions(-) diff --git a/janus_core/calculations/phonons.py b/janus_core/calculations/phonons.py index 5006f474..0718908c 100644 --- a/janus_core/calculations/phonons.py +++ b/janus_core/calculations/phonons.py @@ -90,8 +90,8 @@ class Phonons(BaseCalculation): Keyword arguments to pass to geometry optimizer. Default is {}. n_qpoints : int Number of q-points to sample along generated path, including end points. - Unused if `paths` is specified. Default is 51. - paths : PathLike | None + Unused if `qpoint_file` is specified. Default is 51. + qpoint_file : PathLike | None Path to yaml file with info to generate a path of q-points for band structure. Default is None. dos_kwargs : dict[str, Any] | None @@ -176,7 +176,7 @@ def __init__( minimize: bool = False, minimize_kwargs: dict[str, Any] | None = None, n_qpoints: int = 51, - paths: PathLike | None = None, + qpoint_file: PathLike | None = None, dos_kwargs: dict[str, Any] | None = None, pdos_kwargs: dict[str, Any] | None = None, temp_min: float = 0.0, @@ -248,8 +248,8 @@ def __init__( Keyword arguments to pass to geometry optimizer. Default is {}. n_qpoints : int Number of q-points to sample along generated path, including end points. - Unused if `paths` is specified. Default is 51. - paths : PathLike | None + Unused if `qpoint_file` is specified. Default is 51. + qpoint_file : PathLike | None Path to yaml file with info to generate a path of q-points for band structure. Default is None. dos_kwargs : dict[str, Any] | None @@ -298,7 +298,7 @@ def __init__( self.minimize = minimize self.minimize_kwargs = minimize_kwargs self.n_qpoints = n_qpoints - self.paths = paths + self.qpoint_file = qpoint_file self.dos_kwargs = dos_kwargs self.pdos_kwargs = pdos_kwargs self.temp_min = temp_min @@ -584,14 +584,14 @@ def write_bands( if save_plots is None: save_plots = self.plot_to_file - if self.paths: + if self.qpoint_file: bands_file = self._build_filename("bands.yml.xz", filename=bands_file) - with open(self.paths, encoding="utf8") as paths_file: - paths_info = safe_load(paths_file) + with open(self.qpoint_file, encoding="utf8") as file: + paths_info = safe_load(file) labels = paths_info["labels"] - num_q_points = sum([len(q) for q in paths_info["paths"]]) + num_q_points = sum(len(q) for q in paths_info["paths"]) num_labels = len(labels) assert ( num_q_points == num_labels diff --git a/janus_core/cli/phonons.py b/janus_core/cli/phonons.py index fd764c77..85724fb8 100644 --- a/janus_core/cli/phonons.py +++ b/janus_core/cli/phonons.py @@ -62,11 +62,11 @@ def phonons( Option( help=( "Number of q-points to sample along generated path, including end " - "points. Unused if `paths` is specified" + "points. Unused if `qpoint_file` is specified" ) ), ] = 51, - paths: Annotated[ + qpoint_file: Annotated[ Optional[Path], Option( help=( @@ -166,8 +166,8 @@ def phonons( Whether to calculate and save the band structure. Default is False. n_qpoints : int Number of q-points to sample along generated path, including end points. - Unused if `paths` is specified. Default is 51. - paths : Optional[PathLike] + Unused if `qpoint_file` is specified. Default is 51. + qpoint_file : Optional[PathLike] Path to yaml file with info to generate a path of q-points for band structure. Default is None. dos : bool @@ -322,7 +322,7 @@ def phonons( "minimize": minimize, "minimize_kwargs": minimize_kwargs, "n_qpoints": n_qpoints, - "paths": paths, + "qpoint_file": qpoint_file, "dos_kwargs": dos_kwargs, "pdos_kwargs": pdos_kwargs, "temp_min": temp_min, diff --git a/tests/test_phonons_cli.py b/tests/test_phonons_cli.py index 265ac46e..b66c8d2a 100644 --- a/tests/test_phonons_cli.py +++ b/tests/test_phonons_cli.py @@ -192,8 +192,7 @@ def test_dos(tmp_path): ) assert result.exit_code == 0 assert dos_results.exists() - with open(dos_results, encoding="utf8") as file: - lines = file.readlines() + lines = dos_results.read_text().splitlines() assert lines[1].split()[0] == "-1.0000000000" assert lines[-1].split()[0] == "0.0000000000" @@ -462,7 +461,7 @@ def test_no_carbon(tmp_path): def test_displacement_kwargs(tmp_path): - """Test displacment_kwargs can be set.""" + """Test displacement_kwargs can be set.""" file_prefix_1 = tmp_path / "NaCl_1" file_prefix_2 = tmp_path / "NaCl_2" displacement_file_1 = tmp_path / "NaCl_1-phonopy.yml" @@ -501,21 +500,21 @@ def test_displacement_kwargs(tmp_path): # Check parameters with open(displacement_file_1, encoding="utf8") as file: params = yaml.safe_load(file) - n_displacments_1 = len(params["displacements"]) + n_displacements_1 = len(params["displacements"]) - assert n_displacments_1 == 4 + assert n_displacements_1 == 4 with open(displacement_file_2, encoding="utf8") as file: params = yaml.safe_load(file) - n_displacments_2 = len(params["displacements"]) + n_displacements_2 = len(params["displacements"]) - assert n_displacments_2 == 2 + assert n_displacements_2 == 2 def test_paths(tmp_path): - """Test displacment_kwargs can be set.""" + """Test displacement_kwargs can be set.""" file_prefix = tmp_path / "NaCl" - paths = DATA_PATH / "paths.yml" + qpoint_file = DATA_PATH / "paths.yml" band_results = tmp_path / "NaCl-bands.yml.xz" result = runner.invoke( @@ -526,8 +525,8 @@ def test_paths(tmp_path): DATA_PATH / "NaCl.cif", "--no-hdf5", "--bands", - "--paths", - paths, + "--qpoint-file", + qpoint_file, "--file-prefix", file_prefix, ],