Compatibility interface - handle different types of potentials

[jan-janssen]

Summary by CodeRabbit

• New Features

  • Potentials may be supplied as a string, a table-like row (DataFrame row), or a series; other types now raise a TypeError.

• Tests

  • Test suite updated to validate the new input formats and the TypeError behavior for invalid potential inputs.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

The compatibility lammps_file_interface_function now accepts potential as a str (lookup by name), pandas.DataFrame (uses the first row), or pandas.Series (uses directly); unsupported types raise TypeError. Tests updated to resolve potentials via get_potential_by_name and pass Series/DataFrame forms accordingly.

Changes

Cohort / File(s)	Summary
Type-flexible potential handling pyiron_lammps/compatibility/file.py	Added pandas import and expanded lammps_file_interface_function to accept potential as a str (lookup by name), pandas.DataFrame (use iloc[0]), or pandas.Series (use directly); other types raise TypeError.
Test updates tests/test_compatibility_file.py	Imports pandas and get_potential_by_name; tests now resolve potentials via get_potential_by_name(...) and pass the resolved object (as a Series or as a DataFrame constructed from it) instead of raw strings; updated expectation for invalid potential types to TypeError.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Check correct import and usage of pandas in the module and test environment.
• Verify behavior for empty DataFrame and ensure .iloc[0] is safe.
• Confirm tests construct realistic potential objects and cover string/Series/DataFrame/invalid-type branches.

Possibly related PRs

• Migrate potential interface from atomistics to pyiron_lammps #261 — Adds get_potential_by_name / potential return formats that these changes consume.

Poem

🐰 I nibble strings, I nibble rows,
A Series hop where first-row flows,
I check the type with twitch and cheer,
DataFrame or Series — all welcome here! 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Compatibility interface - handle different types of potentials' accurately summarizes the main change: extending the potential argument handling to accept multiple types (strings, DataFrames, Series) instead of just strings.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch potential

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.56%. Comparing base (b789d91) to head (e0a4789).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #277      +/-   ##
==========================================
+ Coverage   92.51%   92.56%   +0.05%     
==========================================
  Files           8        8              
  Lines         948      955       +7     
==========================================
+ Hits          877      884       +7     
  Misses         71       71              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

tests/test_compatibility_file.py (1)

87-90: Tests now cover Series input; consider adding coverage for DataFrame and invalid types

Passing the result of get_potential_by_name exercises the new pandas.Series branch in lammps_file_interface_function. To fully lock in the new behavior, you might also:

• Add a test that passes a one-row pandas.DataFrame and asserts identical behavior.
• Add a small negative test that passes an unsupported type and asserts a TypeError.

This would make the expanded input handling harder to regress.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b789d91 and 867f882.

📒 Files selected for processing (2)

• pyiron_lammps/compatibility/file.py (2 hunks)
• tests/test_compatibility_file.py (2 hunks)

🧰 Additional context used

🧬 Code graph analysis (2)

tests/test_compatibility_file.py (2)

pyiron_lammps/structure.py (2)

• potential (220-221)
• potential (224-225)

pyiron_lammps/potential.py (1)

• get_potential_by_name (349-355)

pyiron_lammps/compatibility/file.py (2)

pyiron_lammps/structure.py (2)

• potential (220-221)
• potential (224-225)

pyiron_lammps/potential.py (1)

• get_potential_by_name (349-355)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: unittest_matrix (windows-latest, 3.14)

🔇 Additional comments (1)

tests/test_compatibility_file.py (1)

6-6: New get_potential_by_name import is appropriate

Importing get_potential_by_name here is consistent with the new interface that accepts resolved potential objects, and it keeps the test self-contained around how the potential is obtained.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# First, get the file and check its size
wc -l pyiron_lammps/compatibility/file.py

Repository: pyiron/pyiron_lammps

Length of output: 103

---

🏁 Script executed:

#!/bin/bash
# Read the file to see the full context around the import and the function
cat -n pyiron_lammps/compatibility/file.py

Repository: pyiron/pyiron_lammps

Length of output: 10597

---

🌐 Web query:

pandas DataFrame.iloc[0] Series dict-like access API stable 2.3 2024

💡 Result:

Short answer: Yes — df.iloc[0] returns a pandas.Series and you can use label/dict-like access on that Series (e.g. s['col'], s.get('col'), s.keys(), s.items(), s.to_dict()). This is the documented and stable API in pandas 2.3. At the same time note that pandas has deprecated treating integer keys on a Series as positional (use ser.iloc[...] for positional access); that deprecation does not remove label/dict-like access. [1][2][3]

Sources:

• pandas.DataFrame.iloc documentation (shows iloc returns a Series when selecting a single row). [1]
• pandas.Series indexing/API (shows Series methods and label access like getitem, get, keys, items, to_dict). [2]
• Discussion / warning about Series.getitem treating integer keys as positions being deprecated (use .iloc for positions). [3]

---

Update type annotation, docstring, and error message for the potential parameter

The implementation at lines 91–100 correctly supports str, pandas.DataFrame, and pandas.Series inputs, using stable pandas APIs (DataFrame.iloc[0] returning a Series, and dict-like access on Series via ["key"]). However, three documentation gaps remain:

• Type annotation (line 22): Currently declares potential: str, should be Union[str, pandas.DataFrame, pandas.Series] to match the actual behavior.
• Docstring (line 54): Currently describes only the string case ("Name of the LAMMPS potential…"), should document all three accepted types.
• Error handling (line 100): The fallback raises a bare TypeError() with no message; adding context (e.g., "Unsupported type for potential; expected str, DataFrame, or Series") will improve debuggability.

These are non-breaking refinements that align the public API documentation with the actual implementation.

🤖 Prompt for AI Agents

In pyiron_lammps/compatibility/file.py around lines 22, 54, and 100, update the
API docs and error handling to match the implementation: change the function
parameter type annotation from potential: str to potential: Union[str,
pandas.DataFrame, pandas.Series] (add "from typing import Union" at top or use
typing.Union), expand the docstring at line 54 to document all three accepted
types and their meanings/handling (string = potential name, DataFrame = table
where first row is used, Series = dict-like mapping for keys), and replace the
bare TypeError() at line 100 with a descriptive message such as
TypeError("Unsupported type for potential; expected str, pandas.DataFrame, or
pandas.Series") to aid debugging.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

tests/test_compatibility_file.py (1)

145-160: DataFrame-based potential path is well covered; consider minor cleanups

The pattern

potential = get_potential_by_name(...)
potential=pandas.DataFrame({k: [potential[k]] for k in potential.keys()})

correctly constructs a 1-row DataFrame and exercises the pandas.DataFrame branch of lammps_file_interface_function. Functionally this is solid.

Two small polish suggestions:

1. You can simplify the DataFrame construction if you like, e.g.:

potential = get_potential_by_name(...)
potential = pandas.DataFrame([potential])

which produces an equivalent 1-row DataFrame with the same columns.

2. Ruff flags shell_output here as unused. To satisfy linting and make the intent explicit, you can rename it to a throwaway variable:

-        shell_output, parsed_output, job_crashed = lammps_file_interface_function(
+        _shell_output, parsed_output, job_crashed = lammps_file_interface_function(

Both changes are optional but would keep the tests a bit cleaner and lint-free.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6f4ec33 and e0a4789.

📒 Files selected for processing (1)

• tests/test_compatibility_file.py (4 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

tests/test_compatibility_file.py (2)

pyiron_lammps/compatibility/file.py (1)

• lammps_file_interface_function (19-188)

pyiron_lammps/potential.py (1)

• get_potential_by_name (349-355)

🪛 Ruff (0.14.6)

tests/test_compatibility_file.py

149-149: Unpacked variable shell_output is never used

Prefix it with an underscore or any other dummy variable pattern

(RUF059)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

• GitHub Check: unittest_matrix (macos-latest, 3.14)
• GitHub Check: unittest_matrix (windows-latest, 3.14)

🔇 Additional comments (3)

tests/test_compatibility_file.py (3)

5-7: Imports align with new potential-handling paths

Adding pandas and get_potential_by_name matches the new lammps_file_interface_function interface accepting Series/DataFrame. No issues here.

---

40-48: Good negative test for invalid potential type

Asserting a TypeError for potential=1 correctly verifies the new contract that only str, pandas.Series, or pandas.DataFrame are accepted. This nicely pins down the error behavior for unsupported types.

---

94-109: Series-based potential use correctly exercises the new code path

Passing potential=get_potential_by_name(...) makes test_calc_md_npt explicitly hit the pandas.Series branch in lammps_file_interface_function, while still using the same resource path as the function. This is a clean way to test the new interface without changing the physical potential files.