Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add ruff formatter #266

Merged
merged 10 commits into from
May 14, 2024
Merged

add ruff formatter #266

merged 10 commits into from
May 14, 2024

Conversation

jan-janssen
Copy link
Member

@jan-janssen jan-janssen commented May 14, 2024

Summary by CodeRabbit

  • Chores

    • Added .pre-commit-config.yaml for linting and formatting configurations.
  • Refactor

    • Improved code readability by renaming variables and reformatting method calls across various modules.
  • Bug Fixes

    • Corrected error handling in get_minimum_energy_path method.
  • Tests

    • Consolidated and reformatted test cases for better readability and maintainability.
    • Updated dictionary key formatting and assertion statements in several test files.

Copy link

coderabbitai bot commented May 14, 2024

Warning

Rate Limit Exceeded

@jan-janssen has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 19 minutes and 27 seconds before requesting another review.

How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.
Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.
Please see our FAQ for further information.

Commits Files that changed from the base of the PR and between d4f0fb7 and 647542a.

Walkthrough

The recent updates encompass a broad range of changes, including refactoring variable names for clarity, enhancing code readability, and updating string manipulations. Additionally, new configurations for pre-commit hooks have been added, and several functions and methods have been merged into the __all__ lists across various modules. The test files have seen formatting adjustments and improved readability without altering the core logic.

Changes

Files/Groups Change Summaries
.ci_support/release.py Updated conditions and string manipulations in multiple functions.
.pre-commit-config.yaml Added configurations for ruff and ruff-format hooks.
atomistics/calculators/__init__.py, atomistics/calculators/lammps/__init__.py, atomistics/workflows/__init__.py Merged various functions and methods into the __all__ lists.
atomistics/calculators/interface.py, atomistics/calculators/wrapper.py, atomistics/workflows/evcurve/workflow.py, atomistics/workflows/phonons/units.py Removed specific imports.
atomistics/calculators/lammps/helpers.py, atomistics/calculators/lammps/potential.py Updated variable names for clarity.
atomistics/workflows/evcurve/fit.py, atomistics/workflows/evcurve/thermo.py Refactored functions and added exception handling.
tests/... Formatting adjustments, improved readability, and updated assertions across various test files.

🐰

In code, the changes gleam so bright,
Refactored lines, now clear in sight.
Hooks and tests, all set to go,
With ruff and friends, our standards grow.
A rabbit's touch, in every line,
To make our codebase truly shine.
🌟


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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@jan-janssen jan-janssen marked this pull request as draft May 14, 2024 18:44
@jan-janssen jan-janssen added the format_black Launch the pyiron/actions black formatting routine label May 14, 2024
@jan-janssen
Copy link
Member Author

black and ruff have incompatible opinions how to format the code.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 24

Out of diff range and nitpick comments (6)
tests/test_elastic_ase_emt.py (1)

Line range hint 4-4: Remove unused import numpy.

- import numpy as np
tests/test_phonons_ase_emt.py (1)

19-19: Ensure the fmax value is appropriate for your test case.

The fmax value is set to a very small number (0.000001). Verify if this precision is necessary for your test case, as it might increase the computation time.

tests/test_evcurve_ase_emt.py (1)

22-22: Ensure the fmax value is appropriate for your test case.

The fmax value is set to a very small number (0.000001). Verify if this precision is necessary for your test case, as it might increase the computation time.

tests/test_langevin_lammps.py (1)

Line range hint 47-47: Typo in diable_log_file.

- diable_log_file=True,
+ disable_log_file=True,
tests/test_evcurve_eos.py (1)

Line range hint 1-1: Remove unused import numpy.

-import numpy as np
tests/test_quasiharmonic_lammps_functional.py (1)

Line range hint 4-4: Remove unused import numpy.

- import numpy as np
Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits Files that changed from the base of the PR and between 4eda31f and d4f0fb7.
Files selected for processing (55)
  • .ci_support/release.py (3 hunks)
  • .pre-commit-config.yaml (1 hunks)
  • atomistics/calculators/init.py (1 hunks)
  • atomistics/calculators/interface.py (1 hunks)
  • atomistics/calculators/lammps/init.py (1 hunks)
  • atomistics/calculators/lammps/helpers.py (2 hunks)
  • atomistics/calculators/lammps/potential.py (1 hunks)
  • atomistics/calculators/wrapper.py (1 hunks)
  • atomistics/workflows/init.py (1 hunks)
  • atomistics/workflows/evcurve/fit.py (2 hunks)
  • atomistics/workflows/evcurve/thermo.py (1 hunks)
  • atomistics/workflows/evcurve/workflow.py (1 hunks)
  • atomistics/workflows/phonons/units.py (1 hunks)
  • setup.py (1 hunks)
  • tests/test_ase_interface/test_ase_md_mace.py (2 hunks)
  • tests/test_ase_interface/test_ase_md_matgl.py (1 hunks)
  • tests/test_ase_interface/test_elastic_ase_gpaw.py (1 hunks)
  • tests/test_ase_interface/test_evcurve_ase_abinit.py (2 hunks)
  • tests/test_ase_interface/test_evcurve_ase_gpaw.py (1 hunks)
  • tests/test_ase_interface/test_evcurve_ase_mace.py (3 hunks)
  • tests/test_ase_interface/test_evcurve_ase_matgl.py (3 hunks)
  • tests/test_ase_interface/test_evcurve_ase_qe.py (2 hunks)
  • tests/test_ase_interface/test_evcurve_ase_siesta.py (3 hunks)
  • tests/test_ase_interface/test_phonons_ase_gpaw.py (1 hunks)
  • tests/test_ase_interface/test_phonons_ase_mace.py (2 hunks)
  • tests/test_ase_interface/test_phonons_ase_matgl.py (2 hunks)
  • tests/test_ase_interface/test_quasiharmonic_ase_mace.py (2 hunks)
  • tests/test_ase_interface/test_quasiharmonic_ase_matgl.py (2 hunks)
  • tests/test_ase_md_emt.py (3 hunks)
  • tests/test_calc_stress_ase_emt.py (1 hunks)
  • tests/test_calc_stress_lammps.py (3 hunks)
  • tests/test_elastic_ase_emt.py (1 hunks)
  • tests/test_elastic_helper.py (1 hunks)
  • tests/test_elastic_lammps.py (3 hunks)
  • tests/test_elastic_lammps_functional.py (3 hunks)
  • tests/test_evcurve_ase_emt.py (2 hunks)
  • tests/test_evcurve_ase_lammps.py (4 hunks)
  • tests/test_evcurve_eos.py (2 hunks)
  • tests/test_evcurve_lammps.py (4 hunks)
  • tests/test_evcurve_lammps_functional.py (5 hunks)
  • tests/test_evcurve_qe.py (1 hunks)
  • tests/test_hessian_lammps.py (2 hunks)
  • tests/test_lammps_calculator.py (2 hunks)
  • tests/test_lammps_md.py (8 hunks)
  • tests/test_lammps_md_phonons.py (3 hunks)
  • tests/test_langevin_lammps.py (2 hunks)
  • tests/test_molecular_dynamics_thermal_expansion_lammps.py (4 hunks)
  • tests/test_optimize_positions_ase_emt.py (1 hunks)
  • tests/test_optimize_positions_lammps.py (3 hunks)
  • tests/test_phonons_ase_emt.py (2 hunks)
  • tests/test_phonons_lammps.py (5 hunks)
  • tests/test_phonons_lammps_functional.py (5 hunks)
  • tests/test_quasiharmonic_ase_emt.py (2 hunks)
  • tests/test_quasiharmonic_lammps.py (4 hunks)
  • tests/test_quasiharmonic_lammps_functional.py (5 hunks)
Files skipped from review due to trivial changes (18)
  • .pre-commit-config.yaml
  • atomistics/calculators/interface.py
  • atomistics/calculators/lammps/init.py
  • atomistics/calculators/lammps/helpers.py
  • atomistics/workflows/evcurve/workflow.py
  • atomistics/workflows/phonons/units.py
  • setup.py
  • tests/test_ase_interface/test_ase_md_mace.py
  • tests/test_ase_interface/test_ase_md_matgl.py
  • tests/test_ase_interface/test_evcurve_ase_abinit.py
  • tests/test_ase_interface/test_evcurve_ase_mace.py
  • tests/test_ase_interface/test_phonons_ase_mace.py
  • tests/test_ase_interface/test_phonons_ase_matgl.py
  • tests/test_evcurve_qe.py
  • tests/test_lammps_calculator.py
  • tests/test_optimize_positions_lammps.py
  • tests/test_phonons_lammps.py
  • tests/test_phonons_lammps_functional.py
Additional Context Used
Ruff (13)
.ci_support/release.py (4)

3-3: Ambiguous variable name: l


25-25: Ambiguous variable name: l


46-46: Ambiguous variable name: l


49-49: Ambiguous variable name: l

tests/test_elastic_ase_emt.py (1)

4-4: numpy imported but unused

tests/test_evcurve_ase_emt.py (1)

4-4: numpy imported but unused

tests/test_evcurve_ase_lammps.py (2)

4-4: numpy imported but unused


15-15: lammps.lammps imported but unused; consider using importlib.util.find_spec to test for availability

tests/test_evcurve_eos.py (1)

1-1: numpy imported but unused

tests/test_evcurve_lammps.py (1)

4-4: numpy imported but unused

tests/test_hessian_lammps.py (2)

16-16: pylammpsmpi.LammpsASELibrary imported but unused; consider using importlib.util.find_spec to test for availability


19-19: atomistics.calculators.evaluate_with_lammps_library imported but unused; consider using importlib.util.find_spec to test for availability

tests/test_quasiharmonic_lammps_functional.py (1)

4-4: numpy imported but unused

Additional comments not posted (83)
atomistics/workflows/__init__.py (1)

19-28: Ensure all entities in the __all__ list are correctly imported and used within the module.

Verification successful

All entities in the __all__ list are correctly imported and used within the module.

  • ElasticMatrixWorkflow
  • EnergyVolumeCurveWorkflow
  • LangevinWorkflow
  • calc_molecular_dynamics_thermal_expansion
  • optimize_positions
  • optimize_positions_and_volume
  • PhonopyWorkflow
  • QuasiHarmonicWorkflow
Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify all entities in the `__all__` list are correctly imported and used within the module.

# Test: Search for the entities in the module. Expect: All entities should be imported and used.
rg --type python $'ElasticMatrixWorkflow|EnergyVolumeCurveWorkflow|LangevinWorkflow|calc_molecular_dynamics_thermal_expansion|optimize_positions|optimize_positions_and_volume|PhonopyWorkflow|QuasiHarmonicWorkflow'

Length of output: 245



Script:

#!/bin/bash
# Description: Verify all entities in the `__all__` list are correctly imported and used within the module.

# Test: Search for the entities in the module. Expect: All entities should be imported and used.
rg 'ElasticMatrixWorkflow|EnergyVolumeCurveWorkflow|LangevinWorkflow|calc_molecular_dynamics_thermal_expansion|optimize_positions|optimize_positions_and_volume|PhonopyWorkflow|QuasiHarmonicWorkflow' --glob '*.py'

Length of output: 15859

tests/test_optimize_positions_ase_emt.py (1)

21-21: Ensure the fmax value is appropriate for the optimization.

tests/test_calc_stress_ase_emt.py (1)

23-23: Ensure the fmax value is appropriate for the optimization.

Verification successful

Several files in the codebase use different fmax values for optimization. The fmax value of 0.000001 used in tests/test_calc_stress_ase_emt.py is consistent with other test files using the same value.

  • tests/test_quasiharmonic_ase_emt.py
  • tests/test_phonons_ase_emt.py
  • tests/test_optimize_positions_ase_emt.py
  • tests/test_evcurve_ase_lammps.py
  • tests/test_evcurve_ase_emt.py
  • tests/test_elastic_ase_emt.py

This suggests that the fmax value is appropriate for the context in which it is used.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify the `fmax` value used in the optimization.

# Test: Search for the `fmax` value in the test cases. Expect: The `fmax` value should be appropriate for the optimization.
rg --type python $'fmax'

Length of output: 54



Script:

#!/bin/bash
# Description: Verify the `fmax` value used in the optimization.

# Test: Search for the `fmax` value in the test cases. Expect: The `fmax` value should be appropriate for the optimization.
rg --type py 'fmax'

Length of output: 1239

tests/test_ase_interface/test_elastic_ase_gpaw.py (2)

26-26: Ensure the fit_order value is appropriate for the elastic calculations.

Verification successful

The fit_order value of 2 is consistently used in elastic-related test cases, including test_elastic_ase_gpaw.py, indicating it is appropriate for the elastic calculations.

  • tests/test_elastic_lammps_functional.py: fit_order=2
  • tests/test_elastic_lammps.py: fit_order=2
  • tests/test_elastic_ase_emt.py: fit_order=2
  • tests/test_ase_interface/test_elastic_ase_gpaw.py: fit_order=2
Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify the `fit_order` value used in the elastic calculations.

# Test: Search for the `fit_order` value in the test cases. Expect: The `fit_order` value should be appropriate for the elastic calculations.
rg --type python $'fit_order'

Length of output: 59



Script:

#!/bin/bash
# Description: Verify the `fit_order` value used in the elastic calculations.

# Test: Search for the `fit_order` value in the test cases. Expect: The `fit_order` value should be appropriate for the elastic calculations.
rg --type py 'fit_order'

Length of output: 5199


31-31: Ensure the GPAW calculator settings are appropriate for the elastic calculations.

Verification successful

The GPAW calculator settings used in the elastic calculations appear to be appropriate. The settings xc="PBE", mode=PW(300), and kpts=(3, 3, 3) are standard choices for DFT calculations and should be suitable for the intended purpose.

  • tests/test_ase_interface/test_elastic_ase_gpaw.py
  • tests/test_ase_interface/test_phonons_ase_gpaw.py
  • tests/test_ase_interface/test_evcurve_ase_gpaw.py
Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify the GPAW calculator settings used in the elastic calculations.

# Test: Search for the GPAW calculator settings in the test cases. Expect: The GPAW calculator settings should be appropriate for the elastic calculations.
rg --type python $'GPAW'

Length of output: 54



Script:

#!/bin/bash
# Description: Verify the GPAW calculator settings used in the elastic calculations.

# Test: Search for the GPAW calculator settings in the test cases. Expect: The GPAW calculator settings should be appropriate for the elastic calculations.
rg --type py GPAW

Length of output: 628

tests/test_elastic_ase_emt.py (2)

19-19: Improved readability by breaking the arguments into multiple lines.


26-26: Improved readability and consistency by adding a trailing comma.

tests/test_calc_stress_lammps.py (3)

24-24: Standardized string literal format to use double quotes.


37-39: Improved readability and consistency by adding a trailing comma.


38-39: Improved readability and consistency by adding a trailing comma.

tests/test_ase_interface/test_phonons_ase_gpaw.py (1)

33-33: Improved readability and consistency by adding a trailing comma.

tests/test_ase_interface/test_evcurve_ase_qe.py (3)

20-25: Improved readability and consistency by adding a trailing comma.


33-34: Improved readability and consistency by adding a trailing comma.


42-45: Improved readability and consistency by adding a trailing comma.

tests/test_phonons_ase_emt.py (1)

35-41: Good use of assertions to validate the keys in the dictionaries.

These assertions ensure that the expected keys are present in the mesh_dict and dos_dict.

tests/test_ase_interface/test_evcurve_ase_gpaw.py (4)

25-28: Ensure the new arguments are correctly integrated.

Verify that the new arguments fit_type, fit_order, vol_range, and axes are correctly handled within the EnergyVolumeCurveWorkflow class.


34-34: Ensure the GPAW calculator is correctly configured.

Verify that the GPAW calculator is correctly configured with the provided parameters (xc="PBE", mode=PW(300), kpts=(3, 3, 3)).


38-43: Good use of assertions to validate the thermal properties.

These assertions ensure that the temperatures and volumes keys are present and have the expected lengths.


44-51: Good use of np.isclose for floating-point comparisons.

These assertions ensure that the calculated values are close to the expected values within a specified tolerance.

tests/test_evcurve_ase_emt.py (3)

27-30: Ensure the new arguments are correctly integrated.

Verify that the new arguments fit_type, fit_order, vol_range, and axes are correctly handled within the EnergyVolumeCurveWorkflow class.


37-42: Good use of assertions to validate the thermal properties.

These assertions ensure that the temperatures and volumes keys are present and have the expected lengths.


43-45: Good use of assertAlmostEqual for floating-point comparisons.

These assertions ensure that the calculated values are close to the expected values.

tests/test_langevin_lammps.py (2)

12-14: Ensure the imported functions are used correctly.

Verify that the imported functions evaluate_with_lammps_library, get_potential_by_name, and LammpsASELibrary are used correctly within the test method.


30-30: Ensure the potential name and resource path are correct.

Verify that the potential name "1999--Mishin-Y--Al--LAMMPS--ipr1" and the resource path are correct and accessible.

tests/test_ase_interface/test_evcurve_ase_siesta.py (2)

22-29: Consistent use of double quotes for dictionary keys improves readability.


44-47: Consistent use of double quotes for string literals improves readability.

tests/test_evcurve_lammps.py (2)

7-10: Consistent use of double quotes for dictionary keys improves readability.


Line range hint 27-41: Consistent use of double quotes for string literals improves readability.

tests/test_lammps_md_phonons.py (2)

12-14: Consistent use of double quotes for dictionary keys improves readability.


Line range hint 29-65: Consistent use of double quotes for string literals improves readability.

atomistics/calculators/__init__.py (1)

47-72: Properly exporting new functions in the __all__ list ensures they are accessible as intended.

tests/test_ase_interface/test_evcurve_ase_matgl.py (5)

7-10: Import statements are correctly formatted and necessary for the test.


34-34: The addition of ase_optimizer_kwargs improves the configurability of the optimizer.


39-42: The additions of fit_type, fit_order, vol_range, and axes enhance the flexibility and specificity of the workflow configuration.


52-57: The new arguments for get_thermal_properties improve the specificity of the thermal properties calculation.


58-65: The assertions enhance the robustness of the test by verifying the expected outcomes.

.ci_support/release.py (1)

55-69: The code is correctly structured and functional.

tests/test_evcurve_ase_lammps.py (6)

8-11: Import statements are correctly formatted and necessary for the test.


27-27: The addition of cmds is necessary for configuring the LAMMPS calculator.


38-38: The addition of ase_optimizer_kwargs improves the configurability of the optimizer.


43-46: The additions of fit_type, fit_order, vol_range, and axes enhance the flexibility and specificity of the workflow configuration.


60-64: The new arguments for get_thermal_properties improve the specificity of the thermal properties calculation.


66-68: The assertions enhance the robustness of the test by verifying the expected outcomes.

tests/test_evcurve_lammps_functional.py (4)

14-14: Import statements are correctly formatted and necessary for the test.


Line range hint 28-40: The new arguments for get_potential_by_name and evaluate_with_lammps improve the specificity and configurability of the functions.


50-50: The addition of fit_order improves the configurability of the function.


62-65: The assertions enhance the robustness of the test by verifying the expected outcomes.

tests/test_ase_interface/test_quasiharmonic_ase_mace.py (6)

25-25: Reformatted function arguments for better readability. LGTM!


32-32: Reformatted function arguments for better readability. LGTM!


50-52: Reformatted function arguments for better readability. LGTM!


54-55: Reformatted function arguments for better readability. LGTM!


59-60: Reformatted function arguments for better readability. LGTM!


68-69: Reformatted function arguments for better readability. LGTM!

tests/test_evcurve_eos.py (5)

38-40: Reformatted function arguments for better readability. LGTM!


46-48: Reformatted function arguments for better readability. LGTM!


54-56: Reformatted function arguments for better readability. LGTM!


62-66: Reformatted function arguments for better readability. LGTM!


72-74: Reformatted function arguments for better readability. LGTM!

atomistics/calculators/wrapper.py (1)

8-8: Added TYPE_CHECKING import for better type checking. LGTM!

tests/test_molecular_dynamics_thermal_expansion_lammps.py (5)

32-32: Reformatted function arguments for better readability. LGTM!


43-43: Reformatted function arguments for better readability. LGTM!


54-54: Reformatted function arguments for better readability. LGTM!


80-80: Reformatted function arguments for better readability. LGTM!


95-95: Reformatted function arguments for better readability. LGTM!

tests/test_elastic_helper.py (1)

96-96: Ensure the get_symmetry_family_from_SGN function raises a ValueError for SGN=231.

Please verify that get_symmetry_family_from_SGN correctly raises a ValueError for SGN=231. If this is not the intended behavior, update the test accordingly.

tests/test_quasiharmonic_lammps.py (3)

24-26: Reformatting function arguments for better readability.

The reformatting of function arguments improves readability. Ensure that the functionality remains unchanged.


48-53: Reformatting function arguments for better readability.

The reformatting of function arguments improves readability. Ensure that the functionality remains unchanged.


83-84: Reformatting function arguments for better readability.

The reformatting of function arguments improves readability. Ensure that the functionality remains unchanged.

tests/test_quasiharmonic_lammps_functional.py (5)

29-31: Reformatting function arguments for better readability.

The reformatting of function arguments improves readability. Ensure that the functionality remains unchanged.


36-46: Reformatting function arguments for better readability.

The reformatting of function arguments improves readability. Ensure that the functionality remains unchanged.


73-79: Reformatting function arguments for better readability.

The reformatting of function arguments improves readability. Ensure that the functionality remains unchanged.


110-113: Reformatting function arguments for better readability.

The reformatting of function arguments improves readability. Ensure that the functionality remains unchanged.


125-128: Reformatting function arguments for better readability.

The reformatting of function arguments improves readability. Ensure that the functionality remains unchanged.

tests/test_elastic_lammps.py (5)

24-26: Reformatting function arguments for better readability.

The reformatting of function arguments improves readability. Ensure that the functionality remains unchanged.


37-37: Reformatting function arguments for better readability.

The reformatting of function arguments improves readability. Ensure that the functionality remains unchanged.


45-59: Reformatting function arguments for better readability.

The reformatting of function arguments improves readability. Ensure that the functionality remains unchanged.


83-87: Reformatting function arguments for better readability.

The reformatting of function arguments improves readability. Ensure that the functionality remains unchanged.


110-121: Reformatting function arguments for better readability.

The reformatting of function arguments improves readability. Ensure that the functionality remains unchanged.

tests/test_elastic_lammps_functional.py (1)

13-13: Ensure the imported functions evaluate_with_lammps and get_potential_by_name are used in the test cases.

Verification successful

The imported functions evaluate_with_lammps and get_potential_by_name are used in the test cases.

  • get_potential_by_name is used at least once.
  • evaluate_with_lammps is used at least twice.
Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify the usage of `evaluate_with_lammps` and `get_potential_by_name` in the test file.

# Test: Search for the function usage. Expect: Occurrences of the functions in the test cases.
rg --type python 'evaluate_with_lammps|get_potential_by_name' tests/test_elastic_lammps_functional.py

Length of output: 134



Script:

#!/bin/bash
# Description: Verify the usage of `evaluate_with_lammps` and `get_potential_by_name` in the test file.

# Test: Search for the function usage. Expect: Occurrences of the functions in the test cases.
rg --type py 'evaluate_with_lammps|get_potential_by_name' tests/test_elastic_lammps_functional.py

Length of output: 319

tests/test_lammps_md.py (1)

14-14: Ensure the imported function get_potential_by_name is used in the test cases.

Verification successful

The function get_potential_by_name is used in the test cases within tests/test_lammps_md.py, confirming that the import statement is necessary and correctly used.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify the usage of `get_potential_by_name` in the test file.

# Test: Search for the function usage. Expect: Occurrences of the function in the test cases.
rg --type python 'get_potential_by_name' tests/test_lammps_md.py

Length of output: 95



Script:

#!/bin/bash
# Description: Verify the usage of `get_potential_by_name` in the test file.

# Test: Search for the function usage. Expect: Occurrences of the function in the test cases.
rg 'get_potential_by_name' tests/test_lammps_md.py

Length of output: 326

atomistics/calculators/lammps/potential.py (2)

293-297: Renaming the variable l to line improves readability. Ensure the change is consistent throughout the function.


304-304: Adding a default argument env_variables improves the function's usability. Ensure the default values are appropriate for the intended use cases.

atomistics/workflows/evcurve/thermo.py (1)

272-272: Changing the exception from NotImplemented to NotImplementedError is appropriate and follows best practices for indicating unimplemented methods.

Comment on lines +138 to +139
def errfunc(p, x, y, fittype):
return fitfunction(p, x, fittype) - y
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a docstring and type hints to the errfunc function for better readability and maintainability.

def errfunc(p: tuple[float], x: np.ndarray, y: np.ndarray, fittype: str) -> np.ndarray:
    """
    Calculate the error between the fitted function and the actual data.

    Args:
        p (tuple[float]): Fit parameters.
        x (np.ndarray): Volumes.
        y (np.ndarray): Energies.
        fittype (str): Type of fit.

    Returns:
        np.ndarray: Error between the fitted function and the actual data.
    """
    return fitfunction(p, x, fittype) - y

Comment on lines 155 to +157
try:
error.append(np.absolute(pcov[i][i]) ** 0.5)
except:
except TypeError:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Improve the error handling for the pcov calculation by specifying the exception type.

except TypeError:
    error.append(0.00)

@@ -4,12 +4,13 @@
import numpy as np
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove unused import numpy.

- import numpy as np

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.

Suggested change
import numpy as np

Comment on lines +8 to +11
from atomistics.workflows import (
EnergyVolumeCurveWorkflow,
optimize_positions_and_volume,
)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove unused import numpy.

- import numpy as np

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.

Suggested change
from atomistics.workflows import (
EnergyVolumeCurveWorkflow,
optimize_positions_and_volume,
)
from atomistics.workflows import (
EnergyVolumeCurveWorkflow,
optimize_positions_and_volume,
)

@@ -16,7 +16,7 @@ def test_calc_phonons(self):
task_dict=task_dict,
ase_calculator=EMT(),
ase_optimizer=LBFGS,
ase_optimizer_kwargs={"fmax": 0.000001}
ase_optimizer_kwargs={"fmax": 0.000001},
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure consistency in floating-point precision for fmax value.

To maintain consistency, consider using a more readable floating-point precision for fmax value, such as 1e-6 instead of 0.000001.

-            ase_optimizer_kwargs={"fmax": 0.000001},
+            ase_optimizer_kwargs={"fmax": 1e-6},

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.

Suggested change
ase_optimizer_kwargs={"fmax": 0.000001},
ase_optimizer_kwargs={"fmax": 1e-6},

Comment on lines +25 to +27
task_dict = {
"calc_stress": result_dict["structure_with_optimized_positions_and_volume"]
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding more assertions to verify the correctness of the stress calculations.

task_dict = {
    "calc_stress": result_dict["structure_with_optimized_positions_and_volume"]
}
# Additional assertions
self.assertTrue(
    np.allclose(
        stress_pre_dict["stress"],
        stress_post_dict["stress"],
        atol=1e-5
    )
)

Comment on lines 3 to +16
for l in setup_content:
if '==' in l:
lst = l.split('[')[-1].split(']')[0].replace(' ', '').replace('"', '').replace("'", '').split(',')
if "==" in l:
lst = (
l.split("[")[-1]
.split("]")[0]
.replace(" ", "")
.replace('"', "")
.replace("'", "")
.split(",")
)
for dep in lst:
if dep != '\n':
version_lst.append(dep.split('==')[1])
depend_lst.append(dep.split('==')[0])
if dep != "\n":
version_lst.append(dep.split("==")[1])
depend_lst.append(dep.split("==")[0])
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace the ambiguous variable name l with a more descriptive name like line.

-    for l in setup_content:
-        if "==" in l:
-            lst = (
-                l.split("[")[-1]
-                .split("]")[0]
-                .replace(" ", "")
-                .replace('"', "")
-                .replace("'", "")
-                .split(",")
-            )
+    for line in setup_content:
+        if "==" in line:
+            lst = (
+                line.split("[")[-1]
+                .split("]")[0]
+                .replace(" ", "")
+                .replace('"', "")
+                .replace("'", "")
+                .split(",")
+            )

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.

Suggested change
for l in setup_content:
if '==' in l:
lst = l.split('[')[-1].split(']')[0].replace(' ', '').replace('"', '').replace("'", '').split(',')
if "==" in l:
lst = (
l.split("[")[-1]
.split("]")[0]
.replace(" ", "")
.replace('"', "")
.replace("'", "")
.split(",")
)
for dep in lst:
if dep != '\n':
version_lst.append(dep.split('==')[1])
depend_lst.append(dep.split('==')[0])
if dep != "\n":
version_lst.append(dep.split("==")[1])
depend_lst.append(dep.split("==")[0])
for line in setup_content:
if "==" in line:
lst = (
line.split("[")[-1]
.split("]")[0]
.replace(" ", "")
.replace('"', "")
.replace("'", "")
.split(",")
)
for dep in lst:
if dep != "\n":
version_lst.append(dep.split("==")[1])
depend_lst.append(dep.split("==")[0])

Comment on lines +26 to +33
if "dependencies:" in l:
read_flag = True
elif read_flag:
lst = l.replace('-', '').replace(' ', '').replace('\n', '').split("=")
lst = l.replace("-", "").replace(" ", "").replace("\n", "").split("=")
if len(lst) == 2:
depend_lst.append(lst[0])
version_lst.append(lst[1])
return {d:v for d, v in zip(depend_lst, version_lst)}
return {d: v for d, v in zip(depend_lst, version_lst)}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace the ambiguous variable name l with a more descriptive name like line.

-    for l in env_content:
-        if "dependencies:" in l:
+    for line in env_content:
+        if "dependencies:" in line:
-            lst = l.replace("-", "").replace(" ", "").replace("\n", "").split("=")
+            lst = line.replace("-", "").replace(" ", "").replace("\n", "").split("=")

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.

Suggested change
if "dependencies:" in l:
read_flag = True
elif read_flag:
lst = l.replace('-', '').replace(' ', '').replace('\n', '').split("=")
lst = l.replace("-", "").replace(" ", "").replace("\n", "").split("=")
if len(lst) == 2:
depend_lst.append(lst[0])
version_lst.append(lst[1])
return {d:v for d, v in zip(depend_lst, version_lst)}
return {d: v for d, v in zip(depend_lst, version_lst)}
for line in env_content:
if "dependencies:" in line:
read_flag = True
elif read_flag:
lst = line.replace("-", "").replace(" ", "").replace("\n", "").split("=")
if len(lst) == 2:
depend_lst.append(lst[0])
version_lst.append(lst[1])
return {d: v for d, v in zip(depend_lst, version_lst)}

Comment on lines +45 to +50
pattern_dict = {d: d + "==" + v for d, v in version_high_dict.items()}
for l in setup_content:
for k, v in pattern_dict.items():
if v in l:
l = l.replace(v, version_combo_dict[k])
setup_content_new +=l
setup_content_new += l
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace the ambiguous variable name l with a more descriptive name like line.

-    for l in setup_content:
+    for line in setup_content:
-            if v in l:
-                l = l.replace(v, version_combo_dict[k])
-        setup_content_new += l
+            if v in line:
+                line = line.replace(v, version_combo_dict[k])
+        setup_content_new += line

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.

Suggested change
pattern_dict = {d: d + "==" + v for d, v in version_high_dict.items()}
for l in setup_content:
for k, v in pattern_dict.items():
if v in l:
l = l.replace(v, version_combo_dict[k])
setup_content_new +=l
setup_content_new += l
pattern_dict = {d: d + "==" + v for d, v in version_high_dict.items()}
for line in setup_content:
for k, v in pattern_dict.items():
if v in line:
line = line.replace(v, version_combo_dict[k])
setup_content_new += line

Comment on lines +34 to +42
self.assertTrue(
np.isclose(elastic_dict["elastic_matrix"][0, 0], 125.66807354, atol=1e-04)
)
self.assertTrue(
np.isclose(elastic_dict["elastic_matrix"][0, 1], 68.41418321, atol=1e-04)
)
self.assertTrue(
np.isclose(elastic_dict["elastic_matrix"][3, 3], 99.29916329, atol=1e-04)
)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding more assertions to verify the correctness of the elastic matrix calculations.

self.assertTrue(
    np.isclose(elastic_dict["elastic_matrix"][0, 0], 125.66807354, atol=1e-04)
)
self.assertTrue(
    np.isclose(elastic_dict["elastic_matrix"][0, 1], 68.41418321, atol=1e-04)
)
self.assertTrue(
    np.isclose(elastic_dict["elastic_matrix"][3, 3], 99.29916329, atol=1e-04)
)
# Additional assertions
self.assertTrue(
    np.allclose(
        elastic_dict["elastic_matrix"],
        np.array([[125.66807354, 68.41418321, 0, 0, 0, 0],
                  [68.41418321, 125.66807354, 0, 0, 0, 0],
                  [0, 0, 99.29916329, 0, 0, 0],
                  [0, 0, 0, 99.29916329, 0, 0],
                  [0, 0, 0, 0, 99.29916329, 0],
                  [0, 0, 0, 0, 0, 99.29916329]]),
        atol=1e-4
    )
)

@jan-janssen jan-janssen marked this pull request as ready for review May 14, 2024 19:51
@jan-janssen jan-janssen merged commit 17f2f93 into main May 14, 2024
22 of 23 checks passed
@jan-janssen jan-janssen deleted the ruff branch May 14, 2024 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
format_black Launch the pyiron/actions black formatting routine
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants