ruff 0.4.3 auto-fixes
#3808
ruff 0.4.3 auto-fixes
#3808
Conversation
janosh
requested review from
JaGeo,
Andrew-S-Rosen,
shyuep and
mkhorton
as code owners
WalkthroughThis update involves a series of refinements across various Python files, focusing on simplifying and streamlining code. Key changes include updating logger initializations, refining method implementations for clarity and efficiency, and enhancing string operations. The modifications also include updates to version numbers in configuration files, demonstrating a maintenance effort to keep dependencies current. Changes
This table groups files with similar types of changes together to provide a concise overview of the updates made in this patch. Recent Review DetailsConfiguration used: .coderabbit.yaml Files selected for processing (3)
Files skipped from review due to trivial changes (1)
Additional comments not posted (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Out of diff range and nitpick comments (1)
pymatgen/io/pwmat/inputs.py (1)
Line range hint
331-347: Potential issue with loop structure inget_virialmethod.- for tmp_idx in [aim_idx + 1, aim_idx + 2, aim_idx + 3]: - # tmp_strs_lst = - # ['0.8759519000E+01', '0.0000000000E+00', '0.0000000000E+00', - # 'stress', '(eV):', '0.115558E+02', '0.488108E+01', '0.238778E+01'] - tmp_strs_lst = self.strs_lst[tmp_idx].split() - tmp_aim_row_lst = ListLocator.locate_all_lines(strs_lst=tmp_strs_lst, content="STRESS") - if len(tmp_aim_row_lst) == 0: - return None + tmp_strs_lst = [self.strs_lst[tmp_idx].split() for tmp_idx in [aim_idx + 1, aim_idx + 2, aim_idx + 3]] + if not any("STRESS" in sublist for sublist in tmp_strs_lst): + return NoneThis change ensures that the method exits early if "STRESS" is not found in any of the relevant lines, rather than checking each line individually without any effect.
| lines += (f"\t{self.structure.num_sites} atoms\n", "Lattice vector\n") | ||
| for idx in range(3): | ||
| lines.append(f"{lattice.matrix[idx][0]:>15f}{lattice.matrix[idx][1]:>15f}{lattice.matrix[idx][2]:>15f}\n") | ||
| lines.append("Position, move_x, move_y, move_z\n") | ||
| for ii in range(self.structure.num_sites): | ||
| lines.append(f"{int(self.structure.species[ii].Z):>4d}") | ||
| lines.append(f"{self.structure.frac_coords[ii][0]:>15f}") | ||
| lines.append(f"{self.structure.frac_coords[ii][1]:>15f}") | ||
| lines.append(f"{self.structure.frac_coords[ii][2]:>15f}") | ||
| lines.append(" 1 1 1\n") | ||
| for site_idx in range(len(self.structure)): | ||
| lines += ( | ||
| f"{int(self.structure.species[site_idx].Z):>4d}", | ||
| f"{self.structure.frac_coords[site_idx][0]:>15f}", | ||
| f"{self.structure.frac_coords[site_idx][1]:>15f}", | ||
| f"{self.structure.frac_coords[site_idx][2]:>15f}", | ||
| " 1 1 1\n", | ||
| ) |
There was a problem hiding this comment.
Improve consistency in string formatting in AtomConfig class.
- lines += (f"\t{self.structure.num_sites} atoms\n", "Lattice vector\n")
+ lines.append(f"\t{self.structure.num_sites} atoms\nLattice vector\n")
- lines += (
- f"{int(self.structure.species[site_idx].Z):>4d}",
- f"{self.structure.frac_coords[site_idx][0]:>15f}",
- f"{self.structure.frac_coords[site_idx][1]:>15f}",
- f"{self.structure.frac_coords[site_idx][2]:>15f}",
- " 1 1 1\n",
- )
+ lines.append(
+ f"{int(self.structure.species[site_idx].Z):>4d}"
+ f"{self.structure.frac_coords[site_idx][0]:>15f}"
+ f"{self.structure.frac_coords[site_idx][1]:>15f}"
+ f"{self.structure.frac_coords[site_idx][2]:>15f}"
+ " 1 1 1\n"
+ )This change improves the consistency of how lines are added to the list, using append instead of += for single lines and ensuring that all parts of the formatted string are concatenated directly.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| lines += (f"\t{self.structure.num_sites} atoms\n", "Lattice vector\n") | |
| for idx in range(3): | |
| lines.append(f"{lattice.matrix[idx][0]:>15f}{lattice.matrix[idx][1]:>15f}{lattice.matrix[idx][2]:>15f}\n") | |
| lines.append("Position, move_x, move_y, move_z\n") | |
| for ii in range(self.structure.num_sites): | |
| lines.append(f"{int(self.structure.species[ii].Z):>4d}") | |
| lines.append(f"{self.structure.frac_coords[ii][0]:>15f}") | |
| lines.append(f"{self.structure.frac_coords[ii][1]:>15f}") | |
| lines.append(f"{self.structure.frac_coords[ii][2]:>15f}") | |
| lines.append(" 1 1 1\n") | |
| for site_idx in range(len(self.structure)): | |
| lines += ( | |
| f"{int(self.structure.species[site_idx].Z):>4d}", | |
| f"{self.structure.frac_coords[site_idx][0]:>15f}", | |
| f"{self.structure.frac_coords[site_idx][1]:>15f}", | |
| f"{self.structure.frac_coords[site_idx][2]:>15f}", | |
| " 1 1 1\n", | |
| ) | |
| lines.append(f"\t{self.structure.num_sites} atoms\nLattice vector\n") | |
| for idx in range(3): | |
| lines.append(f"{lattice.matrix[idx][0]:>15f}{lattice.matrix[idx][1]:>15f}{lattice.matrix[idx][2]:>15f}\n") | |
| lines.append("Position, move_x, move_y, move_z\n") | |
| for site_idx in range(len(self.structure)): | |
| lines.append( | |
| f"{int(self.structure.species[site_idx].Z):>4d}" | |
| f"{self.structure.frac_coords[site_idx][0]:>15f}" | |
| f"{self.structure.frac_coords[site_idx][1]:>15f}" | |
| f"{self.structure.frac_coords[site_idx][2]:>15f}" | |
| " 1 1 1\n" | |
| ) |
There was a problem hiding this comment.
Actionable comments posted: 1
Out of diff range and nitpick comments (1)
pymatgen/vis/structure_vtk.py (1)
883-883: Consider documenting theDEFAULT_ANIMATED_MOVIE_OPTIONSclass variable.Adding a docstring or comment explaining the purpose and usage of
DEFAULT_ANIMATED_MOVIE_OPTIONSwould enhance code readability and maintainability.
| @@ -392,7 +392,7 @@ def test_labels(self): | |||
| def test_plot(self): | |||
| for ir in self.irs: | |||
| fig = ir.plot(backend="matplotlib") | |||
| assert fig, isinstance(fig, mpl_figure) | |||
| assert fig, isinstance(fig, MplFigure) | |||
There was a problem hiding this comment.
Fix the syntax error in the assertion.
- assert fig, isinstance(fig, MplFigure)
+ assert isinstance(fig, MplFigure)Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| assert fig, isinstance(fig, MplFigure) | |
| assert isinstance(fig, MplFigure) |
simplified code plus some minor logging fixes