merge devel to master to release v0.2.22

[njzjz]

Summary by CodeRabbit

I'll craft concise release notes focusing on the key user-facing changes across the dpdata library:

Release Notes

• Python Version Support

  • Minimum Python version updated from 3.7 to 3.8
  • Removed support for Python 3.7

• New Features

  • Added support for spin data handling in multiple formats (LAMMPS, DeePMD, ABACUS)
  • Enhanced selective dynamics support in VASP POSCAR files
  • Improved handling of magnetic moments and forces in various computational chemistry formats

• Documentation

  • Updated documentation theme from sphinx_rtd_theme to sphinx-book-theme
  • Refined installation and compatibility documentation

• Bug Fixes

  • Improved error handling for force and spin data processing
  • Fixed data type conversions and registrations across different file formats

• Performance

  • Optimized data type handling and conversions
  • Enhanced caching mechanisms for error calculations

[codspeed-hq]

CodSpeed Performance Report

Merging #786 will not alter performance

_{Comparing njzjz:devel-0.2.22 (8ef2646) with master (6387419)}

🎉 Hooray! pytest-codspeed just leveled up to 3.1.2!

A heads-up, this is a breaking change and it might affect your current performance baseline a bit. But here's the exciting part - it's packed with new, cool features and promises improved result stability 🥳!
Curious about what's new? Visit our releases page to delve into all the awesome details about this new version.

Summary

✅ 2 untouched benchmarks

[coderabbitai]

📝 Walkthrough

Walkthrough

This pull request introduces a comprehensive set of changes across the dpdata library, focusing on several key areas:

1. Python Version Compatibility: The project now requires Python 3.8 or higher, with corresponding updates to configuration files and removal of Python 3.7 support.

2. Workflow Improvements: GitHub Actions workflows for benchmarking and testing have been updated to use the astral-sh/setup-uv@v5 action, enhancing environment setup and caching.

3. Data Handling Enhancements: Multiple modules have been modified to improve data type handling, particularly for spin data, forces, and magnetic properties.

4. Documentation and Testing: Updates to documentation, test cases, and configuration files to reflect the new changes and improve test coverage.

Changes

File	Change Summary
.github/workflows/*	Updated UV environment setup, added caching, removed Python 3.7
.pre-commit-config.yaml	Updated pre-commit hook versions
pyproject.toml	Updated Python version requirement, dependencies
README.md, docs/installation.md	Updated Python version support information
dpdata/plugins/*	Added spin data registration, improved force handling
dpdata/system.py	Enhanced type annotations, error handling

Sequence Diagram

sequenceDiagram
    participant User
    participant WorkflowConfig
    participant UVSetup
    participant DependencyCache

    User->>WorkflowConfig: Trigger Workflow
    WorkflowConfig->>UVSetup: Use astral-sh/setup-uv@v5
    UVSetup->>DependencyCache: Check Cache
    alt Cache Hit
        DependencyCache-->>UVSetup: Restore Cached Dependencies
    else Cache Miss
        UVSetup->>DependencyCache: Install Dependencies
        UVSetup->>DependencyCache: Save Cache
    end
    UVSetup-->>WorkflowConfig: Environment Ready

Loading

Possibly related PRs

• breaking: change internal prec of quip/gap/xyz to float64 #777: UV environment setup improvements
• fix: add optional force check #780: Force handling enhancements
• fix: 781 pymatgen structure bug #782: Pymatgen plugin modifications
• breaking: drop python 3.7 #783: Python version support update
• docs: bump minimal dpdata version to 3.8 in README #785: Documentation updates for Python version

Suggested reviewers

• wanghan-iapcm

✨ Finishing Touches

• 📝 Generate Docstrings (Beta)

---

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

• X
• Mastodon
• Reddit
• LinkedIn

🪧 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>, please review it.
  • 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 gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @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.
  • @coderabbitai help me debug CodeRabbit configuration file.

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 using PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration 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.

[coderabbitai]

Actionable comments posted: 8

🧹 Nitpick comments (20)

dpdata/lammps/dump.py (1)

321-331: Handle missing spin data without removing previous spin information.

Currently, when spin data is missing in any frame, the entire spin data is removed from the system. Consider handling missing spin data on a per-frame basis by setting spin values to None or an appropriate default for frames where spin data is missing, instead of discarding all spin data collected so far.

🧰 Tools

🪛 Ruff (0.8.2)

326-326: No explicit stacklevel keyword argument found

(B028)

dpdata/abacus/scf.py (2)

280-280: Simplify multiple isinstance checks for atommag.

You can merge the isinstance checks into a single call for better readability.

Apply this diff:

-    if not (isinstance(atommag, list) or isinstance(atommag, float)):
+    if not isinstance(atommag, (list, float)):

🧰 Tools

🪛 Ruff (0.8.2)

280-280: Multiple isinstance calls for atommag, merge into a single call

Merge isinstance calls for atommag

(SIM101)

---

295-298: Use a ternary operator for concise assignment of mag_norm.

Simplify the if-else block by using a ternary operator.

Apply this diff:

-            if isinstance(atommag, list):
-                mag_norm = np.linalg.norm(atommag)
-            else:
-                mag_norm = atommag
+            mag_norm = np.linalg.norm(atommag) if isinstance(atommag, list) else atommag

🧰 Tools

🪛 Ruff (0.8.2)

295-298: Use ternary operator mag_norm = np.linalg.norm(atommag) if isinstance(atommag, list) else atommag instead of if-else-block

Replace if-else-block with mag_norm = np.linalg.norm(atommag) if isinstance(atommag, list) else atommag

(SIM108)

dpdata/pymatgen/structure.py (1)

14-19: Consider relaxing the PBC validation.

The current implementation raises an error for partial PBC conditions. Consider supporting partial PBC or providing a more informative error message explaining valid alternatives.

-    raise ValueError(f"Partial pbc condition {structure.pbc} is not supported")
+    raise ValueError(
+        f"Partial periodic boundary conditions {structure.pbc} are not supported. "
+        "Please use either fully periodic (True, True, True) or "
+        "non-periodic (False, False, False) conditions."
+    )

tests/test_deepmd_spin.py (1)

18-22: Consider adding more comprehensive spin data validation.

While the current checks verify the presence and shape of spin data, consider adding assertions for:

• Value ranges for spins and force magnitudes
• Consistency between spins and force magnitudes
• Edge cases (zero spins, maximum values)

tests/test_pymatgen_structure.py (1)

31-42: Consider parameterizing precision values.

The precision values (places, e_places, f_places, v_places) are repeated across test classes. Consider moving these to a common base class or configuration.

class PrecisionMixin:
    places = 6
    e_places = 6
    f_places = 6
    v_places = 6

Also applies to: 45-57

tests/test_vasp_poscar_dump.py (2)

37-42: Test setup looks good but could be more robust.

The test correctly sets up the system and performs the conversion. However, consider adding assertions to verify the system state before conversion.

Add assertions to verify the initial state:

 def test_dump_move_flags(self):
     tmp_system = dpdata.System()
     tmp_system.from_vasp_poscar(os.path.join("poscars", "POSCAR.oh.c"))
+    # Verify initial state
+    expected_move = np.array([[[True, True, False], [False, False, False]]])
+    self.assertTrue(np.array_equal(tmp_system["move"], expected_move))
     tmp_system.to_vasp_poscar("tmp.POSCAR")
     self.system = dpdata.System()
     self.system.from_vasp_poscar("tmp.POSCAR")

---

43-50: Consider using file context manager for safer file handling.

The file is properly opened but should use a context manager to ensure proper cleanup.

-        with open("tmp.POSCAR") as f:
+        try:
+            with open("tmp.POSCAR") as f:
+                content = f.read()
+        except IOError as e:
+            self.fail(f"Failed to read POSCAR file: {e}")
-            content = f.read()

dpdata/plugins/lammps.py (1)

97-101: Consider adding error handling for spin registration.

While the implementation is correct, it might benefit from error handling around the spin registration.

         data = dpdata.lammps.dump.system_data(
             lines, type_map, unwrap=unwrap, input_file=input_file
         )
-        register_spin(data)
+        try:
+            register_spin(data)
+        except Exception as e:
+            # Log warning but don't fail if spin registration fails
+            import warnings
+            warnings.warn(f"Failed to register spin data: {e}")
         return data

tests/test_vasp_poscar_to_system.py (1)

22-47: Comprehensive error testing for move flags.

The test class provides good coverage of error cases with clear error messages and proper cleanup.

However, consider adding a positive test case to verify successful move flag handling:

def test_move_flags_success(self):
    system = dpdata.System().from_vasp_poscar(
        os.path.join("poscars", "POSCAR.oh.c")
    )
    system.data["move"] = np.array([[[True, True, True], [False, False, False]]])
    system.to_vasp_poscar(self.tmp_file)
    # Verify the written file
    with open(self.tmp_file) as f:
        content = f.read()
    self.assertIn("T T T", content)
    self.assertIn("F F F", content)

dpdata/vasp/poscar.py (1)

115-128: Consider enhancing move flags validation.

While the validation checks for list type and length, it could be more robust by also validating that each flag is a boolean value.

-            if not isinstance(move_flags, list) or len(move_flags) != 3:
+            if not isinstance(move_flags, list) or len(move_flags) != 3 or not all(isinstance(flag, bool) for flag in move_flags):
                 raise RuntimeError(
-                    f"Invalid move flags: {move_flags}, should be a list of 3 bools"
+                    f"Invalid move flags: {move_flags}, should be a list of 3 boolean values"
                 )

dpdata/unit.py (1)

6-14: Consider adding version control for physical constants.

The physical constants are hardcoded with values from scipy 2018. Consider:

1. Adding a version identifier in the dictionary
2. Documenting the source/reference for these values
3. Adding validation to ensure consistency across updates

tests/test_lammps_spin.py (3)

40-50: Consider adding cleanup in tearDown method.

The tearDown method is defined but currently has a commented-out cleanup. Consider either implementing the cleanup or removing the empty method.

-    def tearDown(self):
-        pass  # if os.path.isfile(self.lmp_coord_name):os.remove(self.lmp_coord_name)
+    def tearDown(self):
+        if os.path.isfile(self.lmp_coord_name):
+            os.remove(self.lmp_coord_name)

---

51-60: Add assertions for spin data in output file.

The test verifies file creation and coordinate reference but doesn't validate the spin data in the output file.

Consider adding assertions to verify the spin data:

     def test_dump_input(self):
         self.tmp_system.to("lammps/lmp", self.lmp_coord_name)
         self.assertTrue(os.path.isfile(self.lmp_coord_name))
         with open(self.lmp_coord_name) as f:
             c = f.read()
+        # Verify spin data in output
+        spin_ref = """    0.6000000000    0.8000000000    0.0000000000    5.0000000000"""
+        self.assertTrue(spin_ref in c)

---

85-106: Add test for invalid spin data format.

The test class should include a test for handling invalid spin data formats.

Consider adding a test method:

def test_read_dump_invalid_spin(self):
    with self.assertRaises(ValueError):
        tmp_system = dpdata.System(
            "lammps/invalid_spin.dump",
            fmt="lammps/dump",
            type_map=["O", "H"],
            input_file="lammps/in.lmp",
        )

dpdata/lammps/lmp.py (2)

130-141: Add input validation for spin data.

The get_spins function should validate the spin data format more thoroughly.

Consider adding more validation:

 def get_spins(lines):
     atom_lines = get_atoms(lines)
     if len(atom_lines[0].split()) < 8:
         return None
+    if not all(len(line.split()) >= 8 for line in atom_lines):
+        raise ValueError("Inconsistent spin data format across atoms")
     spins_ori = []
     spins_norm = []
     for ii in atom_lines:
         iis = ii.split()
+        if not all(isinstance(float(x), float) for x in iis[5:8]):
+            raise ValueError("Invalid spin values")
         spins_ori.append([float(jj) for jj in iis[5:8]])
         spins_norm.append([float(iis[-1])])
     return np.array(spins_ori) * np.array(spins_norm)

---

253-277: Consider extracting spin formatting logic.

The spin formatting logic in from_system_data is complex and could be extracted into a separate function.

Consider refactoring:

def _format_spin_data(spins, spin_norm, idx):
    if spin_norm[idx] != 0:
        return (
            spins[idx][0] / spin_norm[idx],
            spins[idx][1] / spin_norm[idx],
            spins[idx][2] / spin_norm[idx],
            spin_norm[idx],
        )
    return (
        spins[idx][0],
        spins[idx][1],
        spins[idx][2] + 1,
        spin_norm[idx],
    )

tests/test_abacus_stru_dump.py (1)

32-34: Add assertions for file content.

The test only verifies file creation but not its content.

Consider adding content validation:

     def test_dump_stru_without_pporb(self):
         self.system_ch4.to("stru", "STRU_tmp", mass=[12, 1])
         self.assertTrue(os.path.isfile("STRU_tmp"))
+        with open("STRU_tmp") as f:
+            content = f.read()
+        self.assertNotIn("NUMERICAL_ORBITAL", content)
+        self.assertNotIn("NUMERICAL_DESCRIPTOR", content)

dpdata/plugins/ase.py (1)

301-304: Simplify dictionary key checks and improve force handling.

1. The dictionary key checks can be simplified.
2. The force handling is now more robust with proper existence checks.

Apply this diff to simplify the key checks:

-            if "forces" in tmp.keys() and "forces" in dict_frames.keys():
+            if "forces" in tmp and "forces" in dict_frames:
                 dict_frames["forces"] = np.append(
                     dict_frames["forces"], tmp["forces"][0]
                 )
...
-        if "forces" in dict_frames.keys():
+        if "forces" in dict_frames:
             dict_frames["forces"] = dict_frames["forces"].reshape(len(sub_traj), -1, 3)

Also applies to: 313-314

🧰 Tools

🪛 Ruff (0.8.2)

301-301: Use key in dict instead of key in dict.keys()

Remove .keys()

(SIM118)

---

301-301: Use key in dict instead of key in dict.keys()

Remove .keys()

(SIM118)

dpdata/system.py (1)

224-225: Use f-strings for string formatting.

The string formatting can be improved by using f-strings for better readability.

Apply this diff:

-                "Sum of atom_numbs (%d) is not equal to natoms (%d)."  # noqa: UP031
-                % (sum(self.get_atom_numbs()), self.get_natoms())
+                f"Sum of atom_numbs ({sum(self.get_atom_numbs())}) is not equal to natoms ({self.get_natoms()})."
...
-        ret += "\nFrame Numbers     : %d" % self.get_nframes()  # noqa: UP031
-        ret += "\nAtom Numbers      : %d" % self.get_natoms()  # noqa: UP031
+        ret += f"\nFrame Numbers     : {self.get_nframes()}"
+        ret += f"\nAtom Numbers      : {self.get_natoms()}"

Also applies to: 279-280

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6387419 and 8ef2646.

⛔ Files ignored due to path filters (1)

• tests/abacus.spin/OUT.ABACUS-nspin2/running_scf.log is excluded by !**/*.log

📒 Files selected for processing (82)

• .github/workflows/benchmark.yml (1 hunks)
• .github/workflows/test.yml (2 hunks)
• .pre-commit-config.yaml (3 hunks)
• .readthedocs.yaml (1 hunks)
• README.md (1 hunks)
• docs/conf.py (3 hunks)
• docs/installation.md (1 hunks)
• docs/make_format.py (1 hunks)
• docs/rtd_environment.yml (1 hunks)
• dpdata/abacus/md.py (7 hunks)
• dpdata/abacus/relax.py (4 hunks)
• dpdata/abacus/scf.py (14 hunks)
• dpdata/ase_calculator.py (1 hunks)
• dpdata/data_type.py (1 hunks)
• dpdata/deepmd/comp.py (2 hunks)
• dpdata/deepmd/hdf5.py (2 hunks)
• dpdata/deepmd/mixed.py (1 hunks)
• dpdata/deepmd/raw.py (1 hunks)
• dpdata/driver.py (1 hunks)
• dpdata/gaussian/gjf.py (2 hunks)
• dpdata/lammps/dump.py (3 hunks)
• dpdata/lammps/lmp.py (5 hunks)
• dpdata/plugins/abacus.py (5 hunks)
• dpdata/plugins/amber.py (1 hunks)
• dpdata/plugins/ase.py (3 hunks)
• dpdata/plugins/deepmd.py (6 hunks)
• dpdata/plugins/gaussian.py (1 hunks)
• dpdata/plugins/lammps.py (2 hunks)
• dpdata/plugins/n2p2.py (2 hunks)
• dpdata/plugins/pwmat.py (1 hunks)
• dpdata/plugins/pymatgen.py (1 hunks)
• dpdata/plugins/vasp.py (6 hunks)
• dpdata/pwmat/atomconfig.py (2 hunks)
• dpdata/pymatgen/structure.py (1 hunks)
• dpdata/qe/traj.py (2 hunks)
• dpdata/rdkit/sanitize.py (1 hunks)
• dpdata/stat.py (8 hunks)
• dpdata/system.py (8 hunks)
• dpdata/unit.py (1 hunks)
• dpdata/utils.py (1 hunks)
• dpdata/vasp/outcar.py (1 hunks)
• dpdata/vasp/poscar.py (5 hunks)
• dpdata/vasp/xml.py (3 hunks)
• dpdata/xyz/quip_gap_xyz.py (2 hunks)
• pyproject.toml (3 hunks)
• tests/abacus.scf/STRU.ch4 (1 hunks)
• tests/abacus.scf/stru_test (1 hunks)
• tests/abacus.spin/INPUT.scf.nspin2 (2 hunks)
• tests/abacus.spin/STRU.spin (1 hunks)
• tests/comp_sys.py (2 hunks)
• tests/lammps/in.lmp (1 hunks)
• tests/lammps/spin.lmp (1 hunks)
• tests/lammps/traj.dump (1 hunks)
• tests/lammps/traj_partial_spin.dump (1 hunks)
• tests/plugin/pyproject.toml (1 hunks)
• tests/poscars/POSCAR.oh.err1 (1 hunks)
• tests/poscars/POSCAR.oh.err2 (1 hunks)
• tests/poscars/poscar_ref_oh.py (2 hunks)
• tests/pwmat/config_ref_ch4.py (2 hunks)
• tests/pwmat/config_ref_oh.py (2 hunks)
• tests/pymatgen_data/deepmd/type.raw (1 hunks)
• tests/pymatgen_data/deepmd/type_map.raw (1 hunks)
• tests/test_abacus_pw_scf.py (1 hunks)
• tests/test_abacus_spin.py (7 hunks)
• tests/test_abacus_stru_dump.py (3 hunks)
• tests/test_custom_data_type.py (1 hunks)
• tests/test_deepmd_spin.py (1 hunks)
• tests/test_from_pymatgen.py (0 hunks)
• tests/test_lammps_spin.py (1 hunks)
• tests/test_msd.py (1 hunks)
• tests/test_pymatgen_structure.py (1 hunks)
• tests/test_remove_pbc.py (1 hunks)
• tests/test_system_apply_pbc.py (1 hunks)
• tests/test_vasp_poscar_dump.py (1 hunks)
• tests/test_vasp_poscar_to_system.py (1 hunks)
• tests/tmp.deepmd.spin/Fe16-npy/type.raw (1 hunks)
• tests/tmp.deepmd.spin/Fe16-npy/type_map.raw (1 hunks)
• tests/tmp.deepmd.spin/Fe16-raw/box.raw (1 hunks)
• tests/tmp.deepmd.spin/Fe16-raw/coord.raw (1 hunks)
• tests/tmp.deepmd.spin/Fe16-raw/energy.raw (1 hunks)
• tests/tmp.deepmd.spin/Fe16-raw/force.raw (1 hunks)
• tests/tmp.deepmd.spin/Fe16-raw/force_mag.raw (1 hunks)

⛔ Files not processed due to max files limit (7)

• tests/tmp.deepmd.spin/Fe16-raw/spin.raw
• tests/tmp.deepmd.spin/Fe16-raw/type.raw
• tests/tmp.deepmd.spin/Fe16-raw/type_map.raw
• tests/tmp.deepmd.spin/Fe16-raw/virial.raw
• tests/xyz/xyz_unittest.field.xyz
• tests/xyz/xyz_unittest.sort.xyz
• tests/xyz/xyz_unittest.xyz

💤 Files with no reviewable changes (1)

• tests/test_from_pymatgen.py

✅ Files skipped from review due to trivial changes (33)

• tests/tmp.deepmd.spin/Fe16-npy/type_map.raw
• tests/tmp.deepmd.spin/Fe16-npy/type.raw
• tests/tmp.deepmd.spin/Fe16-raw/box.raw
• .readthedocs.yaml
• tests/tmp.deepmd.spin/Fe16-raw/force_mag.raw
• tests/poscars/poscar_ref_oh.py
• tests/test_system_apply_pbc.py
• tests/pymatgen_data/deepmd/type.raw
• tests/pymatgen_data/deepmd/type_map.raw
• tests/pwmat/config_ref_oh.py
• tests/tmp.deepmd.spin/Fe16-raw/force.raw
• dpdata/deepmd/comp.py
• tests/tmp.deepmd.spin/Fe16-raw/energy.raw
• docs/installation.md
• tests/test_custom_data_type.py
• dpdata/plugins/amber.py
• tests/test_remove_pbc.py
• dpdata/deepmd/raw.py
• dpdata/plugins/gaussian.py
• dpdata/vasp/xml.py
• dpdata/rdkit/sanitize.py
• dpdata/data_type.py
• dpdata/gaussian/gjf.py
• dpdata/deepmd/hdf5.py
• dpdata/vasp/outcar.py
• tests/test_msd.py
• tests/tmp.deepmd.spin/Fe16-raw/coord.raw
• tests/poscars/POSCAR.oh.err1
• .pre-commit-config.yaml
• dpdata/qe/traj.py
• dpdata/plugins/n2p2.py
• tests/lammps/traj.dump
• tests/poscars/POSCAR.oh.err2

🧰 Additional context used

🪛 actionlint (1.7.4)

.github/workflows/test.yml

29-29: shellcheck reported issue in this script: SC2102:info:1:26: Ranges can only match single chars (mentioned due to duplicates)

(shellcheck)

🪛 Ruff (0.8.2)

dpdata/abacus/relax.py

79-79: Do not assert False (python -O removes these calls), raise AssertionError()

Replace assert False

(B011)

dpdata/lammps/dump.py

218-218: No explicit stacklevel keyword argument found

(B028)

---

278-278: No explicit stacklevel keyword argument found

(B028)

---

326-326: No explicit stacklevel keyword argument found

(B028)

dpdata/vasp/poscar.py

69-70: Use a single if statement instead of nested if statements

Combine if statements using and

(SIM102)

dpdata/plugins/ase.py

301-301: Use key in dict instead of key in dict.keys()

Remove .keys()

(SIM118)

---

301-301: Use key in dict instead of key in dict.keys()

Remove .keys()

(SIM118)

---

313-313: Use key in dict instead of key in dict.keys()

Remove .keys()

(SIM118)

dpdata/stat.py

119-119: Use of functools.lru_cache or functools.cache on methods can lead to memory leaks

(B019)

---

127-127: Use of functools.lru_cache or functools.cache on methods can lead to memory leaks

(B019)

---

156-156: Use of functools.lru_cache or functools.cache on methods can lead to memory leaks

(B019)

---

169-169: Use of functools.lru_cache or functools.cache on methods can lead to memory leaks

(B019)

dpdata/abacus/scf.py

280-280: Multiple isinstance calls for atommag, merge into a single call

Merge isinstance calls for atommag

(SIM101)

---

295-298: Use ternary operator mag_norm = np.linalg.norm(atommag) if isinstance(atommag, list) else atommag instead of if-else-block

Replace if-else-block with mag_norm = np.linalg.norm(atommag) if isinstance(atommag, list) else atommag

(SIM108)

---

773-773: No explicit stacklevel keyword argument found

(B028)

🔇 Additional comments (74)

dpdata/lammps/dump.py (2)

200-235: Function get_spin_keys implementation looks good.

The function correctly extracts spin keys from the input file with appropriate error handling when the input file is missing or does not contain the expected keys.

🧰 Tools

🪛 Ruff (0.8.2)

218-218: No explicit stacklevel keyword argument found

(B028)

---

238-280: Function get_spin implementation handles spin data extraction appropriately.

The function processes spin information from the dump file, including error handling for potential issues during data extraction.

🧰 Tools

🪛 Ruff (0.8.2)

278-278: No explicit stacklevel keyword argument found

(B028)

dpdata/abacus/scf.py (2)

261-305: Function get_atom_mag_cartesian is implemented correctly.

The function accurately converts magnetic moments from spherical to Cartesian coordinates, handling various input types and edge cases appropriately.

🧰 Tools

🪛 Ruff (0.8.2)

280-280: Multiple isinstance calls for atommag, merge into a single call

Merge isinstance calls for atommag

(SIM101)

---

295-298: Use ternary operator mag_norm = np.linalg.norm(atommag) if isinstance(atommag, list) else atommag instead of if-else-block

Replace if-else-block with mag_norm = np.linalg.norm(atommag) if isinstance(atommag, list) else atommag

(SIM108)

---

570-571: Verify that all references to mag_forces have been updated to force_mags.

Since mag_forces has been renamed to force_mags, ensure that all occurrences in the codebase are updated to prevent potential bugs.

Run the following script to verify:

tests/lammps/in.lmp (1)

1-2: LGTM!

The added compute and dump commands correctly configure the calculation and output of spin properties in the LAMMPS simulation.

tests/abacus.spin/INPUT.scf.nspin2 (1)

2-2: Verify consistency of nspin=2 configuration across test files

The changes to suffix (ABACUS → ABACUS-nspin2) and nspin (4 → 2) parameters appear to be part of a broader update to spin configurations. Please ensure these changes are consistent with related test files and expected test outcomes.

Also applies to: 17-17

✅ Verification successful

Verified: nspin=2 configuration is correctly isolated for specific test case

The nspin=2 configuration in INPUT.scf.nspin2 is intentionally different as it's specifically used by the test_scf_nspin2 test case, while other test files maintain nspin=4 for their respective scenarios. The changes are consistent with the test structure.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for other nspin configurations in test files
rg -l 'nspin.*[24]' tests/

Length of output: 240

---

Script:

#!/bin/bash
# Check nspin values in identified files
echo "=== INPUT files ==="
rg -A 1 'nspin' tests/abacus.spin/INPUT.*

echo -e "\n=== Test file ==="
rg 'nspin.*[24]' tests/test_abacus_spin.py

Length of output: 853

tests/lammps/spin.lmp (1)

1-12: Well-structured LAMMPS test file with spin configurations

The new test file correctly defines:

• System size (2 atoms, 2 types)
• Box dimensions with tilt factors
• Atomic positions with spin parameters

tests/abacus.spin/STRU.spin (1)

20-23: Verify consistency of magnetic property formats

The magnetic properties are specified in different formats across atoms:

• Atom 1: mag 0 0 2
• Atom 2: mag 3
• Atom 3: mag 3 angle1 90
• Atom 4: mag 3 4 0 angle1 90 angle2 90

Please verify if these varying formats are intentional and documented in the test suite.

✅ Verification successful

Magnetic property formats are consistent with test suite

The varying magnetic property formats are intentional and well-documented across the test suite. The formats are consistently used in test files (test_abacus_spin.py), reference files (stru.ref), and have dedicated parsing support (test_abacus_stru_dump.py).

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for similar magnetic property formats in test files
rg -A 1 'mag.*[0-9].*angle' tests/

Length of output: 1951

tests/abacus.scf/STRU.ch4 (1)

21-21: Verify magnetic parameter consistency with CH4 symmetry

The magnetic parameters added to atomic positions:

• Carbon: 1 1 1
• Hydrogens: 1 0 1, 1 0 1, 0 1 1
  These parameters should reflect the tetrahedral symmetry of CH4. Please verify if the varying magnetic parameters for hydrogen atoms are consistent with the expected molecular symmetry.

Also applies to: 26-28

tests/abacus.scf/stru_test (2)

25-25: LGTM: Carbon atom magnetic properties added correctly.

The addition of magnetic properties (mag 0.000000000000 0.000000000000 0.000000000000) to the carbon atom position is consistent with the format and maintains the original occupancy values.

---

29-32: Verify the occupancy pattern changes for hydrogen atoms.

The occupancy values for hydrogen atoms have been modified with a specific pattern:

• H1: 0 0 0 (fully unoccupied)
• H2 & H3: 1 0 1 (partially occupied)
• H4: 0 1 1 (partially occupied)

Please confirm if this occupancy pattern is intentional and physically meaningful for your system.

dpdata/pymatgen/structure.py (2)

27-28: LGTM: New fields added safely.

The addition of "orig" and "nopbc" fields to info_dict maintains backward compatibility while extending functionality.

---

8-8: Verify backward compatibility of symbol extraction change.

The change from site.species_string to ii.specie.symbol might affect systems using older versions of pymatgen. Please ensure this change is compatible across all supported pymatgen versions.

✅ Verification successful

Symbol extraction change is safe to proceed

The codebase shows no usage of the old species_string pattern, and pymatgen dependencies are properly handled through a plugin architecture with appropriate error handling. The change to specie.symbol is consistent with the codebase's current implementation.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check pymatgen version requirements in setup.py or requirements.txt
fd -t f "setup.py|requirements.*" -x rg -l "pymatgen"

Length of output: 53

---

Script:

#!/bin/bash
# Check setup.py content directly
cat setup.py

# Find pymatgen import patterns and both symbol extraction methods
rg "from pymatgen" -A 2
rg "import pymatgen" -A 2
rg "species_string|specie\.symbol" -A 2

Length of output: 2282

tests/test_deepmd_spin.py (1)

24-34: LGTM: File format tests are well-structured.

The tests thoroughly verify both NPY and raw format handling, including proper file creation and cleanup.

Also applies to: 36-44

tests/test_pymatgen_structure.py (1)

9-14: LGTM: Robust module availability checking.

The pymatgen import check is well-implemented with proper exception handling and test skipping.

dpdata/plugins/pwmat.py (1)

34-40: Good improvement in force handling logic.

The changes improve error handling by only including forces when they are available. This prevents potential issues with None values.

dpdata/plugins/lammps.py (2)

17-27: Well-structured spin data registration.

The implementation correctly registers spin data type with appropriate shape and attributes.

---

65-95: Comprehensive docstring with well-documented parameters.

The docstring follows good practices with clear parameter descriptions and return type information.

tests/test_vasp_poscar_to_system.py (1)

17-19: Good test for move flags validation.

The test correctly verifies the expected move flags using numpy array comparison.

dpdata/plugins/pymatgen.py (3)

33-33: LGTM! Clean import statement.

The import statement correctly includes both Structure and Lattice from pymatgen.core.

---

37-38: LGTM! Improved species list construction and PBC handling.

The changes improve the code by:

• Using a more efficient list comprehension for species construction
• Adding explicit PBC handling with a clear default behavior

---

41-41: LGTM! Proper usage of Lattice with PBC.

The code correctly uses the Lattice class to construct the cell with proper PBC handling.

dpdata/plugins/abacus.py (4)

23-25: LGTM! Improved data handling.

The code correctly registers magnetic data after getting frame data from STRU.

---

60-62: LGTM! Proper data type registration for magnetic data.

The code correctly registers magnetic data types for both System and LabeledSystem.

Also applies to: 70-71

---

74-85: LGTM! Well-structured move data registration.

The new register_move_data function follows the same pattern as register_mag_data, maintaining consistency in the codebase.

---

94-94: LGTM! Consistent usage of register_move_data.

The register_move_data function is consistently called across all relevant format classes.

Also applies to: 106-106, 118-118

tests/lammps/traj_partial_spin.dump (1)

1-52: Test data looks good.

This is a well-structured test fixture for LAMMPS trajectory data with and without spin information.

dpdata/stat.py (2)

12-12: LGTM! Improved type annotations.

The return type annotations are now more precise using np.floating[Any] instead of np.float64.

Also applies to: 28-28

---

58-63: LGTM! Improved error messages.

The assertion error messages now clearly indicate the expected type of the system arguments.

dpdata/vasp/poscar.py (5)

7-14: LGTM! Well-implemented flag mapping function.

The move_flag_mapper function correctly handles the conversion of VASP move flags ("T"/"F") to boolean values with proper error handling.

---

38-44: LGTM! Robust selective dynamics handling.

The implementation correctly processes move flags when selective dynamics is enabled, with proper error handling for invalid input formats.

---

55-58: LGTM! Proper reshaping of move flags.

The move flags are correctly reshaped to match the system structure (frames, atoms, dimensions).

---

64-74: LGTM! Clean selective dynamics detection.

The implementation properly handles the "Selective dynamics" header in POSCAR files with case-insensitive matching.

🧰 Tools

🪛 Ruff (0.8.2)

69-70: Use a single if statement instead of nested if statements

Combine if statements using and

(SIM102)

---

99-101: LGTM! Proper handling of selective dynamics header.

The implementation correctly adds the "Selective Dynamics" header when move flags are present.

dpdata/plugins/vasp.py (2)

18-28: LGTM! Well-structured move data registration.

The register_move_data function properly defines and registers the move data type with appropriate dimensions and attributes.

---

107-108: LGTM! Improved force data handling.

The implementation now properly checks for None before assigning force data.

dpdata/utils.py (1)

6-6: LGTM! Simplified typing imports.

Direct import of Literal aligns with the project's Python version requirements.

dpdata/deepmd/mixed.py (1)

18-20: LGTM! Improved assertion formatting.

The assertion message is now more concise while maintaining clarity.

dpdata/unit.py (1)

21-29: LGTM! Clear and well-documented constant definitions.

The constant definitions are clear, with good use of the new scipy_constant_value function. The RYDBERG calculation correctly combines the required constants.

docs/conf.py (2)

27-27: LGTM! Appropriate linter directive added.

The noqa directive for UP031 is correctly applied to the copyright line.

---

47-47: Verify the sphinx_book_theme compatibility.

The theme change from sphinx_rtd_theme to sphinx_book_theme needs verification:

1. Ensure all existing documentation renders correctly
2. Check mobile responsiveness
3. Verify search functionality

Also applies to: 90-90

tests/test_abacus_pw_scf.py (1)

161-161: LGTM! Improved array emptiness check.

The change from assertFalse(system_ch4.data["forces"]) to assertFalse(system_ch4.data["forces"].size) is more explicit and reliable for checking array emptiness.

dpdata/driver.py (1)

169-170: LGTM! Improved error handling for optional data.

The addition of key existence checks prevents KeyError exceptions when handling optional force and virial data.

tests/comp_sys.py (1)

121-123: LGTM! Improved assertion message formatting.

The changes enhance readability by consistently wrapping assertion messages in parentheses.

Also applies to: 127-129, 200-202, 209-211

dpdata/xyz/quip_gap_xyz.py (1)

164-164: LGTM! Enhanced numerical precision.

The change from float32 to float64 provides better numerical precision for scientific computations, which is crucial for accurate molecular dynamics simulations.

Also applies to: 178-181

dpdata/abacus/relax.py (1)

186-186: LGTM! Improved data structure handling.

The changes include:

1. Added support for move parameter in get_coords
2. Renamed mag_forces to force_mags for better clarity
3. Added move data to the output dictionary when available

Also applies to: 220-222

tests/test_abacus_spin.py (3)

19-20: LGTM! Improved test cleanup.

Added proper cleanup of the INPUT file in tearDown, preventing test interference.

---

55-85: LGTM! Comprehensive test coverage for nspin2.

The new test_scf_nspin2 method provides coverage for:

• Energy validation
• Spin vector validation
• Force magnitudes validation
• Data persistence verification

---

194-208: LGTM! Added test coverage for structure file spin reading.

The new test_read_stru_spin method validates:

• Presence of spins in loaded data
• Correct parsing of various spin vector formats
• Accurate conversion of angles to vector components

tests/test_lammps_spin.py (1)

132-144: LGTM! Good error handling test.

The test properly verifies warning generation for incomplete spin data.

dpdata/abacus/md.py (3)

59-61: LGTM! Improved assertion message readability.

The assertion message formatting with parentheses enhances readability.

---

223-225: LGTM! Consistent variable naming.

The renaming from mag_forces to force_mags and addition of move data is consistent with the codebase conventions.

---

170-172: Verify the impact of adding move parameter.

The addition of move parameter to the unpacking might affect other parts of the codebase.

✅ Verification successful

The addition of move parameter is safe and consistent with existing usage

The codebase has two separate get_coords implementations:

• QE implementation returns 4 values (atom_names, natoms, types, coords)
• ABACUS implementation returns 6 values (including move and magmom)

All ABACUS-related files already handle these 6 return values consistently, and the QE implementation remains separate and unaffected.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for other calls to get_coords to ensure they handle the new parameter
ast-grep --pattern 'get_coords($$$)' 

Length of output: 931

---

Script:

#!/bin/bash
# Find implementations of get_coords function
ast-grep --pattern 'def get_coords($$$)'

# Also search for the function name in imports to verify if these are different implementations
rg "import.*get_coords|from.*get_coords"

Length of output: 85

---

Script:

#!/bin/bash
# Look at the content of files where we know get_coords is used
cat dpdata/qe/scf.py | grep -A 20 "def get_coords"
cat dpdata/abacus/scf.py | grep -A 20 "def get_coords"

# Try a broader pattern match for function definition
ast-grep --pattern 'get_coords'

# Search for files containing get_coords to ensure we don't miss any implementations
rg -l "get_coords"

Length of output: 2962

tests/test_abacus_stru_dump.py (2)

58-99: LGTM! Comprehensive error handling tests.

The test cases thoroughly verify different parameter mismatch scenarios with appropriate error messages.

---

153-194: LGTM! Good test coverage for move data.

The test properly verifies the conversion and formatting of move data from VASP format.

docs/make_format.py (1)

7-7: Verify Python version requirements.

The change from typing_extensions to typing for Literal import indicates a minimum Python version requirement of 3.8. Please ensure this aligns with the project's compatibility requirements.

✅ Verification successful

Change from typing_extensions to typing is compatible with project requirements

The project explicitly requires Python >=3.8 and has proper conditional dependencies set up for backward compatibility.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check Python version requirements in project configuration
fd -t f "pyproject.toml|setup.py" -x cat {} | rg -A 5 "python.*version|requires.*python"

Length of output: 656

dpdata/plugins/ase.py (1)

178-180: Improve robustness of force handling.

The conditional check for "forces" key before adding it to results improves error handling and prevents potential KeyErrors.

dpdata/plugins/deepmd.py (1)

21-41: LGTM! Well-structured data type registration.

The implementation correctly:

1. Registers both "spins" and "force_mags" data types
2. Defines appropriate shapes using Axis constants
3. Sets them as optional with required=False
4. Maps to DeePMD-kit names

dpdata/system.py (3)

1271-1272: LGTM! Clean implementation of has_forces method.

The method provides a clean way to check for the existence of forces data.

---

1047-1047: Improve type safety with assertion.

The assertion prevents potential issues with boolean type confusion.

---

1314-1315: Improve robustness of force correction.

The force correction now properly checks for the existence of forces in both systems before applying the correction.

tests/plugin/pyproject.toml (1)

14-14: LGTM! Python version requirement is consistent.

The update to require Python >=3.8 aligns with the project's overall Python version requirements.

.github/workflows/benchmark.yml (1)

16-21: LGTM! Workflow improvements look solid.

Good improvements to the benchmark workflow:

• Using official astral-sh/setup-uv@v5 action is more reliable
• Added caching will improve workflow performance

.github/workflows/test.yml (4)

9-9: LGTM! Fixed Ubuntu version provides better stability.

Using ubuntu-22.04 instead of ubuntu-latest ensures consistent CI environment.

---

12-12: LGTM! Python version matrix aligns with requirements.

Removing Python 3.7 from the test matrix is consistent with the project's move to Python >=3.8.

---

21-27: LGTM! UV setup matches benchmark workflow.

Good improvements to the test workflow:

• Using official astral-sh/setup-uv@v5 action
• Added caching with Python version-specific suffix

---

33-33: LGTM! Codecov action update.

Update to codecov-action@v5 follows good practice of keeping actions up-to-date.

tests/pwmat/config_ref_ch4.py (1)

38-38: LGTM! Linting suppressions are appropriate.

The added # noqa: UP031 comments appropriately suppress string formatting warnings without affecting test functionality.

Also applies to: 60-60

README.md (1)

11-11: LGTM! Documentation update for Python version requirement.

The README correctly reflects the updated minimum Python version requirement of 3.8, which aligns with the changes in pyproject.toml.

dpdata/ase_calculator.py (1)

65-66: LGTM! Improved error handling for forces data.

The added conditional check prevents potential KeyError exceptions when forces data is not available, making the code more robust.

dpdata/pwmat/atomconfig.py (1)

57-57: LGTM! Documented linting suppressions.

The added noqa comments properly document the intentional use of %-formatting for precise numerical output, suppressing UP031 warnings.

Also applies to: 86-86

pyproject.toml (3)

30-30: LGTM! Updated Python version requirement.

The Python version requirement update to >=3.8 aligns with the changes in README and reflects the project's forward movement.

---

39-39: LGTM! Updated entry points configuration.

The change from entry-points.console_scripts to scripts follows modern setuptools conventions.

---

54-54: LGTM! Documentation dependencies update.

Updates to sphinx-book-theme and deepmodeling-sphinx>=0.3.0 improve the documentation tooling.

Also applies to: 57-57

[coderabbitai]

🛠️ Refactor suggestion

Include stacklevel parameter in warnings.warn to improve warning traceability.

Adding stacklevel=2 ensures that warnings point to the relevant line in the user's code.

Apply this diff to fix the issue:

-                    warnings.warn(
-                        f"Warning: spin info is not found in frame {ii}, remove spin info."
-                    )
+                    warnings.warn(
+                        f"Warning: spin info is not found in frame {ii}, remove spin info.",
+                        stacklevel=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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	warnings.warn(
	f"Warning: spin info is not found in frame {ii}, remove spin info."
	)
	warnings.warn(
	f"Warning: spin info is not found in frame {ii}, remove spin info.",
	stacklevel=2
	)

🧰 Tools

🪛 Ruff (0.8.2)

326-326: No explicit stacklevel keyword argument found

(B028)

[coderabbitai]

🛠️ Refactor suggestion

Include stacklevel parameter in warnings.warn call for correct warning location.

Adding stacklevel=2 to the warnings.warn() call helps users trace the origin of the warning more accurately.

Apply this diff to fix the issue:

-            warnings.warn(f"Error processing spin data: {str(e)}")
+            warnings.warn(f"Error processing spin data: {str(e)}", stacklevel=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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	warnings.warn(f"Error processing spin data: {str(e)}")
	return None
	warnings.warn(f"Error processing spin data: {str(e)}", stacklevel=2)
	return None

🧰 Tools

🪛 Ruff (0.8.2)

278-278: No explicit stacklevel keyword argument found

(B028)

[coderabbitai]

🛠️ Refactor suggestion

Add stacklevel parameter to warnings.warn to provide correct warning location.

When issuing warnings with warnings.warn(), it's recommended to include the stacklevel parameter to ensure that the warning message points to the correct location in the user's code where the issue occurred. Without it, the warning may point inside the library code instead of the user's code.

Apply this diff to fix the issue:

-        warnings.warn(f"Input file {inputfile} not found.")
+        warnings.warn(f"Input file {inputfile} not found.", stacklevel=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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	warnings.warn(f"Input file {inputfile} not found.")
	return None
	warnings.warn(f"Input file {inputfile} not found.", stacklevel=2)
	return None

🧰 Tools

🪛 Ruff (0.8.2)

218-218: No explicit stacklevel keyword argument found

(B028)

[coderabbitai]

🛠️ Refactor suggestion

Include stacklevel parameter in warnings.warn for correct warning location.

Adding stacklevel=2 helps users trace the origin of the warning more accurately.

Apply this diff:

-        warnings.warn(
-            "pp_file is not provided, will use empty string for pseudo potential file."
-        )
+        warnings.warn(
+            "pp_file is not provided, will use empty string for pseudo potential file.",
+            stacklevel=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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	warnings.warn(
	"pp_file is not provided, will use empty string for pseudo potential file."
	)
	warnings.warn(
	"pp_file is not provided, will use empty string for pseudo potential file.",
	stacklevel=2
	)

🧰 Tools

🪛 Ruff (0.8.2)

773-773: No explicit stacklevel keyword argument found

(B028)

[coderabbitai]

⚠️ Potential issue

Warning: Potential memory leaks with @lru_cache on methods.

Using @lru_cache on methods can lead to memory leaks as the cache entries are tied to instance methods. Consider one of these alternatives:

1. Move the cached computation to a static method
2. Use @property without caching
3. Implement custom caching with weak references

Example fix using static method:

-    @property
-    @lru_cache
-    def e_errors(self) -> np.ndarray:
+    @staticmethod
+    @lru_cache
+    def _compute_e_errors(system_1: LabeledSystem, system_2: LabeledSystem) -> np.ndarray:
         """Energy errors."""
-        assert isinstance(self.system_1, self.SYSTEM_TYPE)
-        assert isinstance(self.system_2, self.SYSTEM_TYPE)
-        return self.system_1["energies"] - self.system_2["energies"]
+        return system_1["energies"] - system_2["energies"]
+
+    @property
+    def e_errors(self) -> np.ndarray:
+        """Energy errors."""
+        return self._compute_e_errors(self.system_1, self.system_2)

Also applies to: 127-127, 156-156, 169-169

🧰 Tools

🪛 Ruff (0.8.2)

119-119: Use of functools.lru_cache or functools.cache on methods can lead to memory leaks

(B019)

[coderabbitai]

🛠️ Refactor suggestion

Add error handling for missing constants.

The function should handle KeyError gracefully when an invalid key is provided.

 def scipy_constant_value(key: str) -> float:
-    return physical_constants[key]
+    try:
+        return physical_constants[key]
+    except KeyError as e:
+        raise ValueError(f"Unknown physical constant: {key}") from e

📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	def scipy_constant_value(key: str) -> float:
	return physical_constants[key]
	def scipy_constant_value(key: str) -> float:
	try:
	return physical_constants[key]
	except KeyError as e:
	raise ValueError(f"Unknown physical constant: {key}") from e

[coderabbitai]

⚠️ Potential issue

Replace assert False with raise AssertionError.

The use of assert False is discouraged as assertions can be disabled with Python's -O flag. Replace with an explicit exception raise.

-                assert False, "Unrecongnized coordinate type, %s, line:%d" % (  # noqa: UP031
-                    loglines[i].split()[0],
-                    i,
-                )
+                raise AssertionError(
+                    "Unrecongnized coordinate type, %s, line:%d"
+                    % (loglines[i].split()[0], i)
+                )

📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	assert False, "Unrecongnized coordinate type, %s, line:%d" % ( # noqa: UP031
	loglines[i].split()[0],
	i,
	)
	raise AssertionError(
	"Unrecongnized coordinate type, %s, line:%d"
	% (loglines[i].split()[0], i)
	)

🧰 Tools

🪛 Ruff (0.8.2)

79-79: Do not assert False (python -O removes these calls), raise AssertionError()

Replace assert False

(B011)

[coderabbitai]

💡 Codebase verification

Consider updating mamba version constraint

The current constraint mamba <2 is too restrictive as Mamba 2.x is now stable and widely available. Consider using mamba <3 or mamba >=1.5.0,<3 to ensure access to the latest stable features and fixes.

🔗 Analysis chain

LGTM! Version constraints look appropriate.

The version constraints are well-chosen:

• Python <3.13 prevents future compatibility issues
• mamba <2 prevents potential breaking changes in the documentation build

Run this script to verify the mamba version compatibility:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check mamba version compatibility
# Verify if mamba <2 is available in conda-forge

curl -s https://api.anaconda.org/package/conda-forge/mamba | jq -r '.versions[]' | sort -V

Length of output: 1102