Test EntrySet.ground_states and CIF writing in NEBSet.write_input
#3732
Conversation
janosh
added
linting
WalkthroughThe recent updates across various modules in pymatgen focus on enhancing clarity and consistency. Key changes include renaming variables for better understanding, correcting typos in documentation, and refining function signatures. This contributes to improved readability and usability of the code, making it more accessible for developers and researchers working with materials science computations. Changes
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: 7
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (35)
- pymatgen/analysis/adsorption.py (2 hunks)
- pymatgen/analysis/bond_valence.py (4 hunks)
- pymatgen/analysis/chemenv/utils/coordination_geometry_utils.py (2 hunks)
- pymatgen/analysis/ferroelectricity/polarization.py (1 hunks)
- pymatgen/analysis/local_env.py (1 hunks)
- pymatgen/analysis/phase_diagram.py (4 hunks)
- pymatgen/analysis/structure_matcher.py (1 hunks)
- pymatgen/analysis/surface_analysis.py (7 hunks)
- pymatgen/core/lattice.py (1 hunks)
- pymatgen/core/operations.py (2 hunks)
- pymatgen/core/structure.py (2 hunks)
- pymatgen/core/surface.py (5 hunks)
- pymatgen/electronic_structure/bandstructure.py (4 hunks)
- pymatgen/electronic_structure/cohp.py (3 hunks)
- pymatgen/electronic_structure/plotter.py (1 hunks)
- pymatgen/entries/entry_tools.py (1 hunks)
- pymatgen/ext/cod.py (3 hunks)
- pymatgen/ext/matproj.py (4 hunks)
- pymatgen/ext/matproj_legacy.py (5 hunks)
- pymatgen/io/lammps/inputs.py (3 hunks)
- pymatgen/io/lammps/utils.py (1 hunks)
- pymatgen/io/vasp/help.py (2 hunks)
- pymatgen/io/vasp/outputs.py (2 hunks)
- pymatgen/io/vasp/sets.py (3 hunks)
- pymatgen/phonon/plotter.py (1 hunks)
- pymatgen/symmetry/analyzer.py (1 hunks)
- pymatgen/symmetry/kpath.py (1 hunks)
- pymatgen/transformations/advanced_transformations.py (1 hunks)
- pymatgen/transformations/standard_transformations.py (5 hunks)
- pymatgen/util/coord.py (9 hunks)
- pymatgen/util/coord_cython.pyx (1 hunks)
- pymatgen/util/plotting.py (2 hunks)
- tests/electronic_structure/test_cohp.py (1 hunks)
- tests/entries/test_entry_tools.py (3 hunks)
- tests/io/vasp/test_sets.py (1 hunks)
Files not reviewed due to errors (1)
- (no review received)
Files skipped from review due to trivial changes (5)
- pymatgen/analysis/bond_valence.py
- pymatgen/analysis/surface_analysis.py
- pymatgen/io/vasp/help.py
- pymatgen/util/coord_cython.pyx
- tests/entries/test_entry_tools.py
Additional comments not posted (132)
pymatgen/ext/cod.py (2)
93-94: The direct use ofrequests.getwithout error handling could lead to unhandled exceptions if the request fails. Consider adding error handling around this request to manage potential issues such as network errors or unexpected response codes.
115-120: Similar to the previous comment, consider adding error handling for therequests.getcall to manage potential issues such as network errors or unexpected response codes. Additionally, raising an exception within atryblock after issuing a warning may not be the best approach for error handling, as it could lead to uncaught exceptions in the caller's code. Consider returning an error status or using a more controlled way of indicating failure.pymatgen/entries/entry_tools.py (1)
246-249: The refactoring of theground_statesmethod to use dictionary comprehension improves readability and efficiency. This change is a good application of Python's comprehension features to simplify the code.pymatgen/util/coord.py (1)
23-24: The addition of thePbcLikeimport frompymatgen.util.typingand its subsequent use in function signatures is a good practice for type hinting, enhancing code readability and maintainability by making the expected type of thepbcparameter explicit.pymatgen/ext/matproj.py (4)
215-217: Renaming variables for clarity and consistency is a good practice. However, ensure that the renamed variables are consistently used throughout the method to avoid reference errors.
233-234: Using descriptive variable names improves code readability. Ensure that the variableresponseis properly handled in case of errors or unexpected responses to prevent runtime exceptions.
255-256: The renaming of variables to more descriptive names enhances code readability. Similar to previous comments, consider adding error handling for the request to manage potential issues such as network errors or unexpected response codes.
306-308: The use of more descriptive variable names is beneficial for code readability. Ensure that the loop correctly processes and aggregates entries from the response, and consider adding error handling for the request.pymatgen/io/lammps/utils.py (1)
497-497: The renaming of the variable fromstomolin therunmethod is a positive change for readability. However, the specific line change is not visible in the provided snippet. Assuming the renaming is consistently applied throughout the method, this change is approved.pymatgen/analysis/ferroelectricity/polarization.py (1)
128-129: The renaming of the variable fromrtoradiusin theget_nearest_sitefunction is a positive change for readability. This change makes the code more understandable by clearly indicating that the variable represents a radius.pymatgen/core/operations.py (2)
236-237: The change frominvrtoinverseimproves readability by using a more descriptive variable name.
261-275: The updates to variable names in this method significantly improve readability and make the code more understandable.pymatgen/util/plotting.py (1)
714-722: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [694-720]
The
add_fig_kwargsdecorator function is well-structured and provides a comprehensive set of keyword arguments for controlling the display of matplotlib figures. This enhances the usability and flexibility of plotting functions in pymatgen.pymatgen/analysis/adsorption.py (2)
675-675: The renaming ofvertstoverticesimproves clarity and consistency with common geometric terminology. This change enhances code readability.
684-685: Renamingrtoradiusin the context of drawing circles significantly improves the readability of the code by making the purpose of the variable immediately clear.pymatgen/transformations/standard_transformations.py (6)
540-542: The conversion ofreturn_ranked_listton_to_returnusingint()is wrapped in a try-except block forValueError, but it only setsn_to_returnto 1 in the except clause. This approach might not handle non-integer but still numeric values gracefully, such as floats. Consider using a more robust conversion method that can handle floats and integers alike, defaulting to 1 if the conversion fails or if the value is non-numeric.
544-544: The linen_to_return = max(1, n_to_return)ensures that at least one structure is returned, which is a good practice. However, it's important to document this behavior clearly in the method's docstring to inform users that regardless of thereturn_ranked_listvalue, at least one structure will always be returned.
605-605: The variableewald_mis created to hold an instance ofEwaldMinimizer. The naming here is concise, but it could be more descriptive to improve readability. Consider renaming it to something more indicative of its role, such asewald_minimizer.
610-610: The variablen_atomsis introduced to calculate the number of atoms in the structure. This is a straightforward and clear use of thesum()function on the structure's composition. The naming and logic here are appropriate and enhance the readability and maintainability of the code.
630-630: The calculation ofenergy_above_minimumdivides the energy difference byn_atomsto normalize the energy. This approach is logical and provides a clear metric for comparing structures. However, ensure thatn_atomsis always greater than zero to avoid division by zero errors. Adding a check or assertion forn_atoms > 0could improve the robustness of this code segment.
636-636: Returning a slice ofself._all_structuresbased onn_to_returnis a concise and effective way to handle thereturn_ranked_listfunctionality. This implementation is clear and aligns well with the method's intended behavior. It's important to ensure that the method's docstring accurately reflects this behavior for clarity and usability.pymatgen/analysis/chemenv/utils/coordination_geometry_utils.py (5)
345-351: The logic within therectangle_surface_intersectionfunction for calculating the intersection surface has been updated with clearer variable names (f_low_x,f_up_x,min_up,max_lw,zeros,upper,lower). This improves readability and maintainability of the code. However, it's important to ensure that these changes do not alter the function's behavior. The use ofnp.wherefor conditional assignments is efficient and appropriate for vectorized operations on NumPy arrays.
362-369: In thesolid_anglefunction, the variable names have been updated for clarity (origin,r,n,phi). This enhances the readability of the code, making it easier to understand the geometric calculations being performed. The logic and calculations within the function remain unchanged, which is consistent with the PR's objectives. The use of list comprehensions for calculatingrandnis efficient and Pythonic.
342-354: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]
The module docstring provides a clear and concise description of the module's purpose. It's good practice to include such high-level documentation to aid developers and users in understanding the module's role within the larger library.
342-354: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]
Consider adding type hints to function signatures throughout the module to improve code clarity and facilitate static type checking. For example, for the
rectangle_surface_intersectionfunction, type hints could be added to the parameters and return type to explicitly indicate the expected types of inputs and outputs.
342-354: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]
The use of
__author__,__copyright__,__credits__, etc., at the module level is informative but could be moved to documentation or metadata files if the project prefers to centralize such information. This is not a requirement but a suggestion for consistency across the library if applicable.pymatgen/phonon/plotter.py (1)
429-432: Renaming variablesr,g,btored,green,blueimproves readability and is correctly implemented in the_make_colormethod.pymatgen/electronic_structure/bandstructure.py (4)
193-193: The parameterefermiin theBandStructureclass constructor is described as(float): Fermi energyin the docstring. While this change improves clarity by specifying the type and purpose ofefermi, it's also essential to ensure that all references and usages ofefermithroughout the codebase are consistent with this updated description.Verification successful
The usage of
efermiacross the codebase is consistent with its description as a float representing the Fermi energy in the updated docstring. No conflicts or inconsistencies were found regarding its type or interpretation.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for usages of `efermi` to ensure consistency. rg "efermi" --type pyLength of output: 45791
240-241: The methodget_projection_on_elementsnow returns a dictionary in a more structured format, which enhances readability and usability. This change aligns with the PR's objective of improving functionality and readability. Ensure that all calls to this method and related documentation are updated to reflect the new return format.
272-272: Similar to the previous comment, the methodget_projections_on_elements_and_orbitalshas been updated to return a more structured dictionary. This change should be reflected in all usages and documentation related to this method.
302-304: The simplification of the metal detection logic in theis_metalmethod by renaming variables for clarity is a positive change. It's crucial to verify that this logic correctly identifies metals across various test cases, especially since the method is fundamental to determining material properties.Verification successful
The verification process has identified that there are specific unit tests in
tests/electronic_structure/test_bandstructure.pydesigned to assess the functionality of theis_metalmethod within the pymatgen codebase. These tests appear to cover various scenarios, including checks for both metals and non-metals, which suggests a level of testing aimed at ensuring the method's accuracy in identifying metals. Based on this information, it can be concluded that there is an effort to verify the correctness of theis_metalmethod through unit testing.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Suggest writing or verifying existing unit tests for the `is_metal` method to ensure its accuracy. echo "Recommend verifying or adding unit tests for the is_metal method."Length of output: 140
Script:
#!/bin/bash # Search for test files related to the `is_metal` method in the pymatgen codebase fd 'test.*bandstructure.*\.py$'Length of output: 117
Script:
#!/bin/bash # Attempt to find specific tests for the `is_metal` method within identified test files rg 'def test.*is_metal' -A 3Length of output: 1842
pymatgen/io/lammps/inputs.py (1)
557-581: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [560-618]
The modifications in the
from_strmethod of theLammpsInputFileclass improve variable naming for clarity and enhance the logic flow. However, there are a few areas where further improvements can be made:
Variable Naming Consistency: The variable naming improvements are commendable. Ensure that all new variable names are consistently descriptive throughout the method.
Logic Flow Enhancement: The logic flow has been enhanced, particularly in handling comments and stage separation. It's important to ensure that these changes do not introduce any regressions in functionality, especially in edge cases where input strings might be formatted unusually.
Error Handling: Consider adding more robust error handling, especially for cases where the input string might not conform to expected formats. This could include more descriptive error messages or handling specific edge cases gracefully.
Performance Considerations: While not directly related to the changes, it's worth reviewing the method for any potential performance bottlenecks, especially if this method is expected to handle large input strings.
Unit Tests: Given the changes, it's crucial to ensure that there are comprehensive unit tests covering the new logic and any edge cases that might arise from the modifications. This will help prevent regressions in future updates.
Overall, the changes are a positive step towards improving the readability and maintainability of the
from_strmethod. With some additional considerations for error handling, performance, and testing, the method can be further refined.pymatgen/analysis/structure_matcher.py (1)
972-975: The refinement of the return type and the updated documentation for theget_rms_anonymousmethod enhance both the method's usability and clarity. These changes are well-aligned with the PR's objectives of improving functionality and readability.pymatgen/electronic_structure/cohp.py (1)
169-176: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [160-182]
The method
has_antiband_states_below_efermihas been renamed for clarity, and variable names within the method have been updated. Ensure that all references to this method throughout the codebase have been updated to reflect the new name. Additionally, consider adding a brief docstring to explain the parametersspinandlimitfor improved readability and maintainability.tests/electronic_structure/test_cohp.py (1)
82-84: The methodhas_antibnd_states_below_efermihas been renamed tohas_antiband_states_below_efermi. Ensure that all external references to this method are updated accordingly to prevent any broken functionality.Verification successful
The search for the old method name
has_antibnd_states_below_efermidid not yield any results in the repository, indicating that all references to this method have likely been updated to the new namehas_antiband_states_below_efermi. This supports the correctness of the renaming process within the codebase.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for old method name usage in the repository. Expecting no results. rg --type py 'has_antibnd_states_below_efermi'Length of output: 46
pymatgen/ext/matproj_legacy.py (5)
456-459: The renaming of the variable fromresptoresponseimproves readability and is consistent with the PR's objectives.
1106-1111: The renaming of the variable fromresptoresponseimproves readability and is consistent with the PR's objectives.
1132-1137: The renaming of the variable fromresptoresponseimproves readability and is consistent with the PR's objectives.
1160-1165: The renaming of the variable fromresptoresponseimproves readability and is consistent with the PR's objectives.
1258-1263: The renaming of the variable fromresptoresponseimproves readability and is consistent with the PR's objectives.pymatgen/core/lattice.py (5)
1335-1335: The variablelattice_matrixwas renamed tolatt_matrix. This change is minor and does not affect functionality. However, ensure that this renaming is consistent across the entire codebase to avoid reference errors.
1341-1341: The variableris converted to a float directly in the function call. This is a good practice to ensure type consistency, especially when dealing with numerical computations that may be sensitive to type differences.
1332-1344: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [29-29]
The
Latticeclass structure is well-organized, with a clear separation of concerns between constructors, properties, and methods. It inherits fromMSONable, indicating it's designed to be serializable, which is a good practice for data models in pymatgen.
1332-1344: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [72-72]
The properties defined in the
Latticeclass, such aslengths,angles,is_orthogonal, and others, are essential for providing information about the lattice's geometric and physical characteristics. They are implemented correctly and contribute significantly to the class's functionality.
1332-1344: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [72-1341]
The methods within the
Latticeclass, including constructors, utility functions, and specialized lattice operations, are generally well-implemented, adhering to best practices in terms of readability, performance, and maintainability. However, specific attention should be given to ensuring that all external dependencies and method calls are correctly handled, especially in methods involving complex mathematical operations and periodic boundary conditions.tests/io/vasp/test_sets.py (5)
1271-1272: The test methodtest_batch_write_inputinTestFuncclass correctly tests thebatch_write_inputfunction by generating input files for two structures and verifying their existence. Good use of assertions to check the creation of expected files.
1271-1272: TheTestMVLGBSetclass tests theMVLGBSetinput set for both bulk and slab modes. It correctly sets up test cases for both modes and verifies theISIFsetting in the generated INCAR files, as well as the k-point generation. This is a comprehensive test covering the essential aspects of theMVLGBSetfunctionality.
1271-1272: InTestMVLRelax52Set, the tests verify the INCAR settings and the POTCAR functional for theMVLRelax52Set. The tests are well-structured and include checks for user overrides throughuser_incar_settingsanduser_potcar_functional. These tests ensure that theMVLRelax52Setbehaves as expected under various configurations.
1271-1272: TheTestLobsterSetclass provides a thorough test suite for theLobsterSetinput set, covering various scenarios including symmetry settings, smearing types, and custom basis functions. The tests also cover the generation of KPOINTS and the validation of INCAR settings. These tests are crucial for ensuring the correct behavior ofLobsterSetunder different configurations.
1271-1272:TestMPAbsorptionSeteffectively tests theMPAbsorptionSetfor both IPA and RPA modes, including checks for the inheritance of settings from previous calculations, the generation of WAVECAR and WAVEDER files, and the correctness of INCAR settings for each mode. The tests also validate the generation of KPOINTS and the ability to override settings viauser_incar_settings. This comprehensive testing ensures theMPAbsorptionSetis correctly implemented for different absorption calculation modes.pymatgen/core/surface.py (3)
1905-1918: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1879-1917]
The method
center_slabaims to center the slab within the unit cell along the c-axis. However, there's a potential issue with the approach used to handle cases where the slab spills outside the cell from the bottom and into the top. Specifically, the method iterates through all sites and applies a translation based on the conditionany(nn[1] > slab.lattice.c for nn in slab.get_neighbors(site, cutoff_radius)). This condition and the subsequent translation might not correctly handle all scenarios, especially for complex slab geometries or when multiple sites meet the condition simultaneously.A more robust approach might involve calculating the slab's bounding box in fractional coordinates and then applying a uniform translation to center this bounding box within the unit cell. This method would ensure that the entire slab, regardless of its complexity, is centered appropriately.
Consider implementing a more robust centering mechanism that calculates the slab's bounding box in fractional coordinates and then centers this bounding box within the unit cell.
1905-1918: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1879-1917]
The method
repair_broken_bondsattempts to correct undercoordinated atoms by moving them across the slab. This approach, while intuitive, may not always lead to the desired outcome, especially in complex crystal structures or when multiple bonding configurations are possible. It's recommended to add more detailed comments explaining the assumptions and limitations of this method to ensure clarity for future maintainers and users.Consider adding detailed comments within the
repair_broken_bondsmethod to explain the logic, assumptions, and potential limitations of the approach used to repair broken bonds.
1905-1918: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1879-1917]
The function
generate_all_slabsprovides a comprehensive approach to generating both unreconstructed and reconstructed slabs up to a specified Miller index. However, the function could benefit from additional comments explaining the logic behind including reconstructed slabs and the criteria used to filter out equivalent slabs. This would improve readability and maintainability, especially for users unfamiliar with the underlying concepts.Consider adding detailed comments within the
generate_all_slabsfunction to explain the process of including reconstructed slabs, the use of symmetry operations to filter out equivalent slabs, and any assumptions made regarding the structure and its symmetry.pymatgen/symmetry/kpath.py (8)
1683-1692: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [63-78]
The
__init__method ofKPathBaseis well-structured and clear.
1683-1692: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [80-116]
The property methods in
KPathBaseare straightforward and provide clear access to the structure, lattice, reciprocal lattice, and kpath.
1683-1692: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [123-187]
The
__init__method ofKPathSetyawanCurtarolois comprehensive and handles various lattice types. Note that ignoring 'magmom' in site properties for this convention might impact the calculation results for magnetic materials.
1683-1692: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [189-1085]
The methods for generating paths based on lattice types in
KPathSetyawanCurtaroloare well-implemented and specific to the Setyawan and Curtarolo convention.
1683-1692: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1087-1169]
The
__init__method ofKPathSeekis comprehensive and correctly implements the Hinuma et al. convention. Consider adding more detailed documentation about the handling of magnetic moments for clarity.
1683-1692: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1171-1204]
The
_trans_sc_to_Hinmethod inKPathSeekcorrectly implements the transformation logic for sub-classifications according to the Hinuma et al. convention.
1683-1692: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1206-1689]
The
__init__method ofKPathLatimerMunrois comprehensive and correctly implements the Latimer and Munro convention. Consider adding more detailed documentation about the handling of magnetic moments and the calculation of magnetic symmetry operations for clarity.
1683-1692: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1691-2687]
The helper methods in
KPathLatimerMunroare well-implemented and specific to the Latimer and Munro convention.pymatgen/transformations/advanced_transformations.py (9)
232-232: Consider improving the clarity and robustness of species handling inMultipleSubstitutionTransformation, especially regarding the use of dummy species and string manipulation for species names. Utilizing more explicit data structures or methods provided by pymatgen for species manipulation could enhance code readability and reduce the potential for errors.
232-232: Ensure that external dependencies, such asmatglfor M3GNet calculations inEnumerateStructureTransformation, are documented and checked for availability before use. This can help prevent runtime errors and improve the user experience by providing clear error messages or fallback options.
232-232: Consider enhancing the handling of magnetic species and dummy species inMagOrderingTransformation, especially in the context of magnetic ordering. Utilizing more explicit data structures or methods provided by pymatgen for species manipulation in a magnetic context could improve code readability and reduce the potential for errors.
232-232: Improve error handling in theFindCodopantfunction for cases where the target species has no ionic radius. Providing a clearer error message or a fallback mechanism could enhance usability and provide clearer guidance to users.
232-232: Enhance documentation or provide examples for theDopingTransformationclass to clarify the doping process and the use of various parameters. This could help users better understand the capabilities of the class and how to effectively use it for their specific doping scenarios.
232-232: Consider enhancing the handling of species and disorder mappings in theDisorderOrderedTransformationclass, especially in the context of generating disordered structures from ordered inputs. Utilizing more explicit data structures or methods provided by pymatgen for species manipulation in the context of disorder could improve code readability and reduce the potential for errors.
232-232: Enhance documentation or provide examples for theCubicSupercellTransformationclass to clarify the process of generating nearly cubic supercells and the use of various parameters. This could help users better understand the capabilities of the class and how to effectively use it for their specific scenarios.
232-232: Enhance documentation or provide examples for theSQSTransformationclass to clarify the process of generating special quasi-random structures (SQS) and the use of various parameters. This could help users better understand the capabilities of the class and how to effectively use it for their specific scenarios.
232-232: Add documentation regarding the dependency on thehiphivepackage for theMonteCarloRattleTransformationclass. Informing users about this external requirement can help ensure they have the necessary setup to use this transformation effectively.pymatgen/io/vasp/sets.py (28)
2363-2374: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [141-141]
The
@deprecateddecorator is used for theget_vasp_inputmethod, indicating it will be removed in future versions. Ensure that any usage of this method in your codebase is updated to useget_input_setinstead.
2363-2374: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [141-141]
The implementation of
potcar_symbolsproperty correctly handles both dictionary and non-dictionary settings for POTCAR configuration.
2363-2374: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [141-141]
The
potcarproperty correctly generates aPotcarobject and includes a warning mechanism for potential issues with POTCAR selection.
2363-2374: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [141-141]
The
write_inputmethod is well-implemented, supporting a range of features for generating VASP input files.
2363-2374: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [141-141]
The
__post_init__method inDictSetperforms important validation and setup based on the provided configuration. Ensure that the validation logic aligns with the expected configurations for your calculations.
2363-2374: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [141-141]
The
structureproperty inDictSetcorrectly handles structure reduction, sorting, and special cases for PSPs.
2363-2374: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [141-141]
The
incarproperty inDictSetis complex but correctly handles the generation of anIncarobject based on various conditions and settings.
2363-2374: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [141-141]
MITRelaxSetcorrectly specifies configurations for MIT relaxation calculations, relying on predefined settings.
2363-2374: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [141-141]
MPRelaxSetcorrectly specifies configurations for Materials Project relaxation calculations, using predefined settings.
2363-2374: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [141-141]
MPScanRelaxSetcorrectly handles configurations for SCAN relaxation calculations, including support for SCAN+rVV10.
2363-2374: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [141-141]
MPStaticSetcorrectly sets up static calculations with considerations for dielectric and polarization calculations.
2363-2374: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [141-141]
MPNonSCFSetcorrectly implements logic for different non-SCF calculation modes, including k-point and INCAR settings.
2363-2374: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [141-141]
MPSOCSetcorrectly sets up spin-orbit coupling calculations with considerations for spin polarization and theSAXISparameter.
2363-2374: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [141-141]
MPMDSetcorrectly sets up molecular dynamics simulations with considerations for temperature, time steps, and spin polarization.
2363-2374: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [141-141]
MITNEBSetcorrectly sets up nudged elastic band calculations, including handling of a series of structures.
2363-2374: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [141-141]
MITMDSetcorrectly sets up molecular dynamics simulations with specific settings for MIT projects.
2363-2374: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [141-141]
MVLGWSetcorrectly implements logic for different modes of GW calculations, including appropriate settings based on the mode.
2363-2374: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [141-141]
MVLSlabSetcorrectly sets up slab calculations with appropriate k-point settings and other considerations.
2363-2374: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [141-141]
MVLGBSetcorrectly sets up grain boundary calculations with appropriate k-point settings and other considerations.
2363-2374: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [141-141]
MVLRelax52Setcorrectly sets up relaxation calculations using VASP's recommended PAW potentials.
2363-2374: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [141-141]
LobsterSetcorrectly sets up calculations for Lobster analysis with appropriate settings and considerations.
2363-2374: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [141-141]
MPAbsorptionSetcorrectly implements logic for different modes of frequency-dependent dielectric calculations, including appropriate settings based on the mode.
2363-2374: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [141-141]
MPMDSetcorrectly sets up molecular dynamics simulations with specific settings for the Materials Project.
2363-2374: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [141-141]
MITNEBSetcorrectly sets up nudged elastic band calculations, including handling of a series of structures.
2363-2374: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [141-141]
MITMDSetcorrectly sets up molecular dynamics simulations with specific settings for MIT projects.
2363-2374: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [141-141]
MVLGWSetcorrectly implements logic for different modes of GW calculations, including appropriate settings based on the mode.
2363-2374: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [141-141]
MVLSlabSetcorrectly sets up slab calculations with appropriate k-point settings and other considerations.
2363-2374: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [141-141]
MVLGBSetcorrectly sets up grain boundary calculations with appropriate k-point settings and other considerations.pymatgen/analysis/phase_diagram.py (2)
3761-3761: The functionuniquelinesis defined outside of any class. Consider moving utility functions like this into a separate module or incorporating them as static methods within a relevant class to improve modularity and maintainability.
3761-3761: Theuniquelinesfunction could benefit from a more descriptive name that clearly indicates its purpose, such asget_unique_line_segments. This would improve code readability and maintainability.pymatgen/io/vasp/outputs.py (32)
3159-3167: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [14-14]
The
__future__import for annotations is good practice for forward compatibility.
3159-3167: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [15-15]
The import of
datetimeis appropriate for handling date and time-related operations.
3159-3167: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [16-16]
The import of
itertoolsis suitable for efficient looping.
3159-3167: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [17-17]
The import of
loggingis standard for logging messages in Python applications.
3159-3167: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [18-18]
The import of
mathis necessary for mathematical operations.
3159-3167: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [19-19]
The import of
osis standard for interacting with the operating system.
3159-3167: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [20-20]
The import of
reis standard for regular expression operations.
3159-3167: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [21-21]
The import of
warningsis appropriate for issuing warnings to the developer.
3159-3167: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [22-22]
The import of
xml.etree.ElementTree as ETis suitable for parsing and creating XML data.
3159-3167: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [23-23]
The import of
collections.defaultdictis useful for creating dictionaries with default values.
3159-3167: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [24-24]
The import of
collections.abc.Iterableis appropriate for type checking iterable objects.
3159-3167: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [25-25]
The import of
dataclasses.dataclassis suitable for creating data classes in Python.
3159-3167: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [26-26]
The import of
glob.globis standard for file pattern matching.
3159-3167: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [27-27]
The import of
io.StringIOis appropriate for in-memory text streams.
3159-3167: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [28-28]
The import of
pathlib.Pathis modern practice for handling filesystem paths.
3159-3167: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [29-29]
The import of
typingmodule is good for type hinting.
3159-3167: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [30-30]
The import of
numpy as npis standard for numerical operations in Python.
3159-3167: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [31-31]
The import of
monty.iois specific to the Monty library, which is a dependency of Pymatgen.
3159-3167: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [32-32]
The import of
monty.jsonis specific to the Monty library for JSON operations.
3159-3167: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [33-33]
The import of
monty.os.pathis specific to the Monty library for path operations.
3159-3167: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [34-34]
The import of
monty.reis specific to the Monty library for regular expression operations.
3159-3167: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [35-35]
The import of
numpy.testing.assert_allcloseis appropriate for unit tests to compare array elements.
3159-3167: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [37-37]
The import of core Pymatgen classes and functions is necessary for the functionality provided in this file.
3159-3167: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [38-38]
The import of
pymatgen.core.units.unitizedis suitable for handling units in physical quantities.
3159-3167: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [39-39]
The import of electronic structure related classes from Pymatgen is appropriate for handling electronic structure data.
3159-3167: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [40-40]
The import of
pymatgen.entries.computed_entriesis necessary for handling computed entries in materials informatics.
3159-3167: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [41-41]
The import of VASP input related classes from Pymatgen is suitable for handling VASP input files.
3159-3167: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [42-42]
The import of
pymatgen.io.wannier90is specific to handling Wannier90 related data.
3159-3167: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [43-43]
The import of utility functions from Pymatgen is appropriate for various helper operations.
3159-3167: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [44-44]
The import of numerical utility functions from Pymatgen is suitable for numerical operations.
3159-3167: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [46-46]
The import of
typing_extensionsis necessary for extended type hinting capabilities.
3159-3167: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [48-48]
The logger configuration is standard practice for logging messages in Python applications.
| red = [0.5 * (red[i] + red[i + 1]) for i in range(n_seg)] | ||
| green = [0.5 * (green[i] + green[i + 1]) for i in range(n_seg)] | ||
| blue = [0.5 * (blue[i] + blue[i + 1]) for i in range(n_seg)] | ||
| alpha = np.ones(n_seg, float) * alpha | ||
| lc = LineCollection(seg, colors=list(zip(red, green, blue, alpha)), linewidth=2, linestyles=linestyles) |
There was a problem hiding this comment.
The calculation of the average color values for the _rgbline function seems to be done in a non-vectorized manner, which could be optimized for performance by leveraging NumPy's vectorized operations.
- red = [0.5 * (red[i] + red[i + 1]) for i in range(n_seg)]
- green = [0.5 * (green[i] + green[i + 1]) for i in range(n_seg)]
- blue = [0.5 * (blue[i] + blue[i + 1]) for i in range(n_seg)]
- alpha = np.ones(n_seg, float) * alpha
+ red = 0.5 * (red[:-1] + red[1:])
+ green = 0.5 * (green[:-1] + green[1:])
+ blue = 0.5 * (blue[:-1] + blue[1:])
+ alpha = np.full(n_seg, alpha)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.
| red = [0.5 * (red[i] + red[i + 1]) for i in range(n_seg)] | |
| green = [0.5 * (green[i] + green[i + 1]) for i in range(n_seg)] | |
| blue = [0.5 * (blue[i] + blue[i + 1]) for i in range(n_seg)] | |
| alpha = np.ones(n_seg, float) * alpha | |
| lc = LineCollection(seg, colors=list(zip(red, green, blue, alpha)), linewidth=2, linestyles=linestyles) | |
| red = 0.5 * (red[:-1] + red[1:]) | |
| green = 0.5 * (green[:-1] + green[1:]) | |
| blue = 0.5 * (blue[:-1] + blue[1:]) | |
| alpha = np.full(n_seg, alpha) | |
| lc = LineCollection(seg, colors=list(zip(red, green, blue, alpha)), linewidth=2, linestyles=linestyles) |
| redu_comp = entry.composition.reduced_composition | ||
| products.append(f"{fmt(c / redu_comp.num_atoms * factor)} {redu_comp.reduced_formula}") |
There was a problem hiding this comment.
The variable redu_comp is assigned but never used within the loop or elsewhere. This could be a remnant of a previous version of the code or an oversight. If it's not needed, consider removing it to clean up the code.
- redu_comp = entry.composition.reduced_compositionCommittable 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.
| redu_comp = entry.composition.reduced_composition | |
| products.append(f"{fmt(c / redu_comp.num_atoms * factor)} {redu_comp.reduced_formula}") | |
| products.append(f"{fmt(c / redu_comp.num_atoms * factor)} {redu_comp.reduced_formula}") |
| response = subprocess.check_output(["mysql", "-u", "cod_reader", "-h", self.url, "-e", sql, "cod"]) | ||
| return response.decode("utf-8") |
There was a problem hiding this comment.
The subprocess call to execute a MySQL query directly from Python can be a security risk if not properly sanitized, especially if user input is ever incorporated into the SQL query. Consider using a dedicated library for database access that supports parameterized queries to mitigate SQL injection risks.
| @@ -229,7 +229,7 @@ def apply_transformation(self, structure: Structure, return_ranked_list: bool | | |||
| dummy_sp: self.r_fraction, | |||
| } | |||
| } | |||
| trans = SubstitutionTransformation(mapping) # type: ignore | |||
| trans = SubstitutionTransformation(mapping) # type: ignore[arg-type] | |||
There was a problem hiding this comment.
The type ignore comment # type: ignore[arg-type] on the instantiation of SubstitutionTransformation suggests a potential type mismatch or an issue that could be addressed more appropriately. It's important to ensure that the types passed to functions or classes match their expected input types to maintain type safety and code clarity. If there's a known issue with the type checking that cannot be resolved, it would be beneficial to document the reason for ignoring the type check here for future reference and clarity.
| dummy_sp: self.r_fraction, | ||
| } | ||
| } | ||
| trans = SubstitutionTransformation(mapping) # type: ignore | ||
| trans = SubstitutionTransformation(mapping) # type: ignore[arg-type] | ||
| dummy_structure = trans.apply_transformation(structure) | ||
| if self.charge_balance_species is not None: | ||
| cbt = ChargeBalanceTransformation(self.charge_balance_species) |
There was a problem hiding this comment.
📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [31-31]
Consider adding explicit handling or documentation for the case where specie.oxi_state is zero to avoid potential division by zero errors in ChargeBalanceTransformation.
| radius = 1 | ||
|
|
||
| x1 = dist_bins[idx] | ||
| x2 = dist_bins[idx + 1] | ||
|
|
||
| if dist_bins[idx] == 1: | ||
| area1 = 0.25 * math.pi * r**2 | ||
| area1 = 0.25 * math.pi * radius**2 | ||
| else: | ||
| area1 = 0.5 * ((x1 * math.sqrt(r**2 - x1**2)) + (r**2 * math.atan(x1 / math.sqrt(r**2 - x1**2)))) | ||
| area1 = 0.5 * ( | ||
| (x1 * math.sqrt(radius**2 - x1**2)) + (radius**2 * math.atan(x1 / math.sqrt(radius**2 - x1**2))) | ||
| ) | ||
|
|
||
| area2 = 0.5 * ((x2 * math.sqrt(r**2 - x2**2)) + (r**2 * math.atan(x2 / math.sqrt(r**2 - x2**2)))) | ||
| area2 = 0.5 * ((x2 * math.sqrt(radius**2 - x2**2)) + (radius**2 * math.atan(x2 / math.sqrt(radius**2 - x2**2)))) | ||
|
|
||
| return (area1 - area2) / (0.25 * math.pi * r**2) | ||
| return (area1 - area2) / (0.25 * math.pi * radius**2) |
There was a problem hiding this comment.
The method _semicircle_integral in the CrystalNN class is incorrectly split across multiple lines, which breaks the Python syntax.
- radius = 1
-
- x1 = dist_bins[idx]
- x2 = dist_bins[idx + 1]
-
- if dist_bins[idx] == 1:
- area1 = 0.25 * math.pi * radius**2
- else:
- area1 = 0.5 * (
- (x1 * math.sqrt(radius**2 - x1**2)) + (radius**2 * math.atan(x1 / math.sqrt(radius**2 - x1**2)))
- )
-
- area2 = 0.5 * ((x2 * math.sqrt(radius**2 - x2**2)) + (radius**2 * math.atan(x2 / math.sqrt(radius**2 - x2**2))))
-
- return (area1 - area2) / (0.25 * math.pi * radius**2)
+ # The method implementation should be continuous without line breaks in the logic.
+ # Example correction (implementation might vary):
+ radius = 1
+ x1 = dist_bins[idx]
+ x2 = dist_bins[idx + 1]
+ if dist_bins[idx] == 1:
+ area1 = 0.25 * math.pi * radius**2
+ else:
+ area1 = 0.5 * ((x1 * math.sqrt(radius**2 - x1**2)) + (radius**2 * math.atan(x1 / math.sqrt(radius**2 - x1**2))))
+ area2 = 0.5 * ((x2 * math.sqrt(radius**2 - x2**2)) + (radius**2 * math.atan(x2 / math.sqrt(radius**2 - x2**2))))
+ return (area1 - area2) / (0.25 * math.pi * radius**2)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.
| radius = 1 | |
| x1 = dist_bins[idx] | |
| x2 = dist_bins[idx + 1] | |
| if dist_bins[idx] == 1: | |
| area1 = 0.25 * math.pi * r**2 | |
| area1 = 0.25 * math.pi * radius**2 | |
| else: | |
| area1 = 0.5 * ((x1 * math.sqrt(r**2 - x1**2)) + (r**2 * math.atan(x1 / math.sqrt(r**2 - x1**2)))) | |
| area1 = 0.5 * ( | |
| (x1 * math.sqrt(radius**2 - x1**2)) + (radius**2 * math.atan(x1 / math.sqrt(radius**2 - x1**2))) | |
| ) | |
| area2 = 0.5 * ((x2 * math.sqrt(r**2 - x2**2)) + (r**2 * math.atan(x2 / math.sqrt(r**2 - x2**2)))) | |
| area2 = 0.5 * ((x2 * math.sqrt(radius**2 - x2**2)) + (radius**2 * math.atan(x2 / math.sqrt(radius**2 - x2**2)))) | |
| return (area1 - area2) / (0.25 * math.pi * r**2) | |
| return (area1 - area2) / (0.25 * math.pi * radius**2) | |
| # The method implementation should be continuous without line breaks in the logic. | |
| # Example correction (implementation might vary): | |
| radius = 1 | |
| x1 = dist_bins[idx] | |
| x2 = dist_bins[idx + 1] | |
| if dist_bins[idx] == 1: | |
| area1 = 0.25 * math.pi * radius**2 | |
| else: | |
| area1 = 0.5 * ((x1 * math.sqrt(radius**2 - x1**2)) + (radius**2 * math.atan(x1 / math.sqrt(radius**2 - x1**2)))) | |
| area2 = 0.5 * ((x2 * math.sqrt(radius**2 - x2**2)) + (radius**2 * math.atan(x2 / math.sqrt(radius**2 - x2**2)))) | |
| return (area1 - area2) / (0.25 * math.pi * radius**2) |
| for struct_matcher in to_move: | ||
| species.append(struct_matcher.species) | ||
| for group in selection: | ||
| if s in group: | ||
| frac_coords.append(s.frac_coords) | ||
| if struct_matcher in group: | ||
| frac_coords.append(struct_matcher.frac_coords) | ||
| break | ||
| else: | ||
| # Move unselected atom to the opposite surface. | ||
| frac_coords.append(s.frac_coords + [0, 0, shift]) # noqa: RUF005 | ||
| frac_coords.append(struct_matcher.frac_coords + [0, 0, shift]) # noqa: RUF005 |
There was a problem hiding this comment.
In the method get_tasker2_slabs, the variable struct_matcher is used as an iterator variable in a loop but is also defined as a StructureMatcher instance later in the code (lines 291-292). This dual usage of the variable name within the same method scope can lead to confusion and potential bugs. Consider renaming the iterator variable to something more descriptive and different from struct_matcher to avoid this ambiguity.
- for struct_matcher in to_move:
+ for site_to_move in to_move:And replace all occurrences of struct_matcher within the loop accordingly.
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.
| for struct_matcher in to_move: | |
| species.append(struct_matcher.species) | |
| for group in selection: | |
| if s in group: | |
| frac_coords.append(s.frac_coords) | |
| if struct_matcher in group: | |
| frac_coords.append(struct_matcher.frac_coords) | |
| break | |
| else: | |
| # Move unselected atom to the opposite surface. | |
| frac_coords.append(s.frac_coords + [0, 0, shift]) # noqa: RUF005 | |
| frac_coords.append(struct_matcher.frac_coords + [0, 0, shift]) # noqa: RUF005 | |
| for site_to_move in to_move: | |
| species.append(site_to_move.species) | |
| for group in selection: | |
| if site_to_move in group: | |
| frac_coords.append(site_to_move.frac_coords) | |
| break | |
| else: | |
| # Move unselected atom to the opposite surface. | |
| frac_coords.append(site_to_move.frac_coords + [0, 0, shift]) # noqa: RUF005 |
plus code clean up and single-letter variable renaming for readability
Summary by CodeRabbit