Migrate to file based interface #193
Conversation
Add copyright to tests
Black: The Uncompromising Code Formatter
Lmp average position
Fix LAMMPS pressure
* Move hydrostatic test into function * Add setup to control tests * Break apart for readability * Better variable names
* Add an interface to the lammps move fix. * Add tests * Sam-improved docstring * Sam-safe group ids * Update tests * Abstract out control to allow either move linear or set force * Add Tobias' warning * Rename method * Extend warning. * Add coordinate warning to forces fix too * Force int arange * Fix URL * Docstring warnigns to warnings warnings * Update pyiron/lammps/control.py Co-authored-by: Jan Janssen <[email protected]> Co-authored-by: Jan Janssen <[email protected]>
Fix notebooks for pyiron_atomistic
Fix notebooks for pyiron_atomistic
Update copyright to 2021
Update copyright to 2021
reordering of input items in Lammps
* Bump numpy from 1.19.5 to 1.20.0 Bumps [numpy](https://github.com/numpy/numpy) from 1.19.5 to 1.20.0. - [Release notes](https://github.com/numpy/numpy/releases) - [Changelog](https://github.com/numpy/numpy/blob/master/doc/HOWTO_RELEASE.rst.txt) - [Commits](numpy/numpy@v1.19.5...v1.20.0) Signed-off-by: dependabot-preview[bot] <[email protected]> * Update environment.yml * Numpy debug (#61) * Try fix numpy bug * If parent does not exist return None * next try * revert * extened * Update type comparisons * numpy float type updates * Debug warnings * more debugging * Increase number of allowed warnings * Update environment.yml * Update setup.py Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> Co-authored-by: Jan Janssen <[email protected]>
automatic triclinic when potentially triclinic
automatic triclinic when potentially triclinic
Document energy_pot_per_atom
Lammps units
Lammps units
Extending the units class
Only force skew when tensions are applied
Interactive units
Lammps style full - fix numpy warnings
Make codebase black
Make codebase black
Make codebase black
Adjust n_print to be at most max_iter in calc_minimize
Bump pyiron-base from 0.5.4 to 0.5.5
Modify where number of bond types is chosen from
ruff fixes
ruff fixes
Lammps units
ruff fixes
Debug read only
ruff fixes
Lammps velocitiy handling
ruff fixes
Increase lammps parsing robustness
add logic to treat multiple log steps
Fix notebooks for pyiron_atomistic
|
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 16 minutes and 39 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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. 📒 Files selected for processing (4)
WalkthroughThe changes introduce new files and update existing configurations and workflows across the repository. A new Python script in the CI support handles dependency modifications in the Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Repo as Repository
participant CI as CI/CD Pipeline
participant Bot as Auto-merge Bot
Dev->>Repo: Push changes / Create PR
Repo->>CI: Trigger pipeline workflow
CI->>CI: Run code formatting (black)
alt Formatting Fails
CI->>CI: Run auto-fix job (black_fix)
end
CI->>CI: Execute tests (coverage, minimal, pip_check, unit tests)
CI->>Bot: Identify bot-generated PRs (Dependabot/Pre-commit)
Bot->>Repo: Auto-merge PR (if conditions met)
sequenceDiagram
participant User as User
participant LC as LammpsControl
participant LAMMPS as LAMMPS Engine
User->>LC: Initialize simulation parameters
LC->>LC: Load default configuration and process inputs
User->>LC: Request simulation type (minimization/MD/VC-SGC)
LC->>LC: Convert pressure and set up simulation commands
LC->>LAMMPS: Execute simulation
LAMMPS-->>LC: Return simulation results
LC-->>User: Output processed simulation data
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (24)
pyiron_atomistics/lammps/output.py (3)
69-69: Use dictionary membership test without.keys().
Replaceif "computes" in dump_dict.keys():with a direct membership test, e.g.if "computes" in dump_dict:for cleaner and more Pythonic code.- if "computes" in dump_dict.keys(): + if "computes" in dump_dict:🧰 Tools
🪛 Ruff (0.8.2)
69-69: Use
key in dictinstead ofkey in dict.keys()Remove
.keys()(SIM118)
96-96: Addstacklevelto warnings.
When emitting a warning, consider addingstacklevel=2(or appropriate) to produce more informative file/line references.- warnings.warn("LAMMPS warning: No log.lammps output file found.") + warnings.warn("LAMMPS warning: No log.lammps output file found.", stacklevel=2)🧰 Tools
🪛 Ruff (0.8.2)
96-96: No explicit
stacklevelkeyword argument found(B028)
281-281: Usenot in dump.computesdirectly.
Remove the explicit call to.keys()inif kk not in dump.computes.keys():.- if kk not in dump.computes.keys(): + if kk not in dump.computes:🧰 Tools
🪛 Ruff (0.8.2)
281-281: Use
key not in dictinstead ofkey not in dict.keys()Remove
.keys()(SIM118)
pyiron_atomistics/lammps/structure.py (6)
114-115: Combine nested conditions.
You can merge the twoifstatements into one to simplify logic:if np.abs(d_a) < self.acc and d_a < 0: ...🧰 Tools
🪛 Ruff (0.8.2)
114-115: Use a single
ifstatement instead of nestedifstatementsCombine
ifstatements usingand(SIM102)
123-124: Addstacklevelargument to warnings.
Inwarnings.warn(...), includestacklevelto help track the source of the warning.- warnings.warn( + warnings.warn( "Skewed lammps cells should have PBC == True in all directions!", + stacklevel=2 )🧰 Tools
🪛 Ruff (0.8.2)
123-123: No explicit
stacklevelkeyword argument found(B028)
427-427: Rename unused loop variable.
i_shellis not used within the loop. Rename it to_or remove if unneeded.- for i_shell, ib_shell_lst in enumerate(b_lst): + for _, ib_shell_lst in enumerate(b_lst):🧰 Tools
🪛 Ruff (0.8.2)
427-427: Loop control variable
i_shellnot used within loop bodyRename unused
i_shellto_i_shell(B007)
491-492: Combine nested conditions.
Use a singleifstatement to check both conditions.if el_1_list is not None and len(el_1_list) > 0: ...🧰 Tools
🪛 Ruff (0.8.2)
491-492: Use a single
ifstatement instead of nestedifstatements(SIM102)
529-532: Use a ternary operator for brevity.
You can replace the if-else block with a one-liner:num_bond_types = 0 if len(bond_type_lst) == 0 else int(np.max(bond_type_lst))🧰 Tools
🪛 Ruff (0.8.2)
529-532: Use ternary operator
num_bond_types = 0 if len(bond_type_lst) == 0 else int(np.max(bond_type_lst))instead ofif-else-blockReplace
if-else-block withnum_bond_types = 0 if len(bond_type_lst) == 0 else int(np.max(bond_type_lst))(SIM108)
533-536: Use a ternary operator fornum_angle_types.
Similarly, replace the if-else block with a concise expression:num_angle_types = 0 if len(angle_type_lst) == 0 else int(np.max(angle_type_lst))🧰 Tools
🪛 Ruff (0.8.2)
533-536: Use ternary operator
num_angle_types = 0 if len(angle_type_lst) == 0 else int(np.max(angle_type_lst))instead ofif-else-blockReplace
if-else-block withnum_angle_types = 0 if len(angle_type_lst) == 0 else int(np.max(angle_type_lst))(SIM108)
pyiron_atomistics/lammps/control.py (10)
241-241: Use direct membership checks.
Replace the usage of.keys()inif self["units"] not in LAMMPS_UNIT_CONVERSIONS.keys():with a simpler membership check.- if self["units"] not in LAMMPS_UNIT_CONVERSIONS.keys(): + if self["units"] not in LAMMPS_UNIT_CONVERSIONS:🧰 Tools
🪛 Ruff (0.8.2)
241-241: Use
key not in dictinstead ofkey not in dict.keys()Remove
.keys()(SIM118)
411-411: Use direct membership checks.
Same improvement for membership testing:- if self["units"] not in LAMMPS_UNIT_CONVERSIONS.keys(): + if self["units"] not in LAMMPS_UNIT_CONVERSIONS:🧰 Tools
🪛 Ruff (0.8.2)
411-411: Use
key not in dictinstead ofkey not in dict.keys()Remove
.keys()(SIM118)
421-421: Attach original exception cause.
When re-raising inside anexcept, consider usingraise ... from ...for cleaner exception chaining.except KeyError as e: raise NotImplementedError() from e🧰 Tools
🪛 Ruff (0.8.2)
421-421: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
425-425: Addstacklevelto warnings.
Includingstacklevel=2(or a suitable value) helps identify the source.- warnings.warn( + warnings.warn( "WARNING: `delta_temp` is deprecated, please use `temperature_damping_timescale`.", + stacklevel=2 )🧰 Tools
🪛 Ruff (0.8.2)
425-425: No explicit
stacklevelkeyword argument found(B028)
434-434: Addstacklevelto warnings.
Likewise, specifystacklevelin thiswarnings.warn(...)call.🧰 Tools
🪛 Ruff (0.8.2)
434-434: No explicit
stacklevelkeyword argument found(B028)
532-532: Addstacklevelto warnings.
Help locate the code path that triggered the message:- warnings.warn("Temperature not set; Langevin ignored.") + warnings.warn("Temperature not set; Langevin ignored.", stacklevel=2)🧰 Tools
🪛 Ruff (0.8.2)
532-532: No explicit
stacklevelkeyword argument found(B028)
633-633: Use direct membership checks.
Remove the overhead of.keys():- if self["units"] not in LAMMPS_UNIT_CONVERSIONS.keys(): + if self["units"] not in LAMMPS_UNIT_CONVERSIONS:🧰 Tools
🪛 Ruff (0.8.2)
633-633: Use
key not in dictinstead ofkey not in dict.keys()Remove
.keys()(SIM118)
760-760: Addstacklevelto warnings.
Include astacklevelargument in thewarnings.warncall to help trace the origin.🧰 Tools
🪛 Ruff (0.8.2)
760-760: No explicit
stacklevelkeyword argument found(B028)
839-839: Addstacklevelto warnings.
Again, clarifying the source callsite of the warning is recommended.🧰 Tools
🪛 Ruff (0.8.2)
839-839: No explicit
stacklevelkeyword argument found(B028)
861-861: Addstacklevelto warnings.
Similar improvement to aid debugging.🧰 Tools
🪛 Ruff (0.8.2)
861-861: No explicit
stacklevelkeyword argument found(B028)
tests/lammps/test_structure.py (2)
42-45: Investigate and document the project coupling issue.The comment indicates a workaround for project coupling issues that are not well understood. This should be investigated and properly documented to prevent future maintenance challenges.
54-76: Enhance test coverage for velocity handling.While the test covers basic scenarios, consider adding test cases for:
- Zero velocities
- Very large/small velocities
- Random velocities
- Velocity scaling
Would you like me to generate additional test cases to improve coverage?
pyiron_atomistics/lammps/units.py (1)
231-240: Optimize dictionary lookup and warning.Consider these improvements:
- Use
label in quantity_dictinstead oflabel in quantity_dict.keys()for better performance- Add
stacklevelparameter to the warning for better error trackingApply this diff:
- if label in quantity_dict.keys(): + if label in quantity_dict: return np.array( array * self.lammps_to_pyiron(quantity_dict[label]), dtype_dict[label] ) else: warnings.warn( message="Warning: Couldn't determine the LAMMPS to pyiron unit conversion type of quantity " - "{}. Returning un-normalized quantity".format(label) + "{}. Returning un-normalized quantity".format(label), + stacklevel=2 )🧰 Tools
🪛 Ruff (0.8.2)
231-231: Use
key in dictinstead ofkey in dict.keys()Remove
.keys()(SIM118)
236-236: No explicit
stacklevelkeyword argument found(B028)
tests/lammps/test_control.py (2)
98-98: Useis Nonefor None comparisons.Replace
== Nonewithis Nonefor more idiomatic Python code.Apply this diff:
- and tmp[3] == tmp[4] == tmp[5] == None + and tmp[3] is None and tmp[4] is None and tmp[5] is NoneAlso applies to: 103-103
🧰 Tools
🪛 Ruff (0.8.2)
98-98: Comparison to
Noneshould becond is NoneReplace with
cond is None(E711)
156-175: Add positive test assertions.While error cases are well-covered, consider adding assertions to verify the correct behavior when valid inputs are provided.
Add assertions after line 160 to verify that the fix was correctly applied with the given IDs and velocities.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
pyiron_atomistics/lammps/control.py(1 hunks)pyiron_atomistics/lammps/output.py(1 hunks)pyiron_atomistics/lammps/structure.py(1 hunks)pyiron_atomistics/lammps/units.py(1 hunks)tests/lammps/test_control.py(1 hunks)tests/lammps/test_structure.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
tests/lammps/test_control.py
98-98: Comparison to None should be cond is None
Replace with cond is None
(E711)
103-103: Comparison to None should be cond is None
Replace with cond is None
(E711)
pyiron_atomistics/lammps/units.py
231-231: Use key in dict instead of key in dict.keys()
Remove .keys()
(SIM118)
236-236: No explicit stacklevel keyword argument found
(B028)
pyiron_atomistics/lammps/output.py
69-69: Use key in dict instead of key in dict.keys()
Remove .keys()
(SIM118)
96-96: No explicit stacklevel keyword argument found
(B028)
178-178: Using .strip() with multi-character strings is misleading
(B005)
281-281: Use key not in dict instead of key not in dict.keys()
Remove .keys()
(SIM118)
328-328: Ambiguous variable name: l
(E741)
329-329: Ambiguous variable name: l
(E741)
415-415: No explicit stacklevel keyword argument found
(B028)
456-456: No explicit stacklevel keyword argument found
(B028)
pyiron_atomistics/lammps/control.py
241-241: Use key not in dict instead of key not in dict.keys()
Remove .keys()
(SIM118)
411-411: Use key not in dict instead of key not in dict.keys()
Remove .keys()
(SIM118)
421-421: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
425-425: No explicit stacklevel keyword argument found
(B028)
434-434: No explicit stacklevel keyword argument found
(B028)
532-532: No explicit stacklevel keyword argument found
(B028)
633-633: Use key not in dict instead of key not in dict.keys()
Remove .keys()
(SIM118)
760-760: No explicit stacklevel keyword argument found
(B028)
839-839: No explicit stacklevel keyword argument found
(B028)
861-861: No explicit stacklevel keyword argument found
(B028)
pyiron_atomistics/lammps/structure.py
114-115: Use a single if statement instead of nested if statements
Combine if statements using and
(SIM102)
123-123: No explicit stacklevel keyword argument found
(B028)
427-427: Loop control variable i_shell not used within loop body
Rename unused i_shell to _i_shell
(B007)
491-492: Use a single if statement instead of nested if statements
(SIM102)
529-532: Use ternary operator num_bond_types = 0 if len(bond_type_lst) == 0 else int(np.max(bond_type_lst)) instead of if-else-block
Replace if-else-block with num_bond_types = 0 if len(bond_type_lst) == 0 else int(np.max(bond_type_lst))
(SIM108)
533-536: Use ternary operator num_angle_types = 0 if len(angle_type_lst) == 0 else int(np.max(angle_type_lst)) instead of if-else-block
Replace if-else-block with num_angle_types = 0 if len(angle_type_lst) == 0 else int(np.max(angle_type_lst))
(SIM108)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build (macos-latest, 3.12)
- GitHub Check: build
- GitHub Check: build (macos-latest, 3.12, osx-64-py-3-12, /Users/runner/miniconda3/envs/my-env)
🔇 Additional comments (2)
pyiron_atomistics/lammps/output.py (1)
178-178: Re-assess.lstrip()with a multi-character string.
line.lstrip("ITEM: ATOMS")may inadvertently remove characters in an order-insensitive manner if they appear in any sequence. To avoid any unintended truncation, consider explicitly handling the prefix.🧰 Tools
🪛 Ruff (0.8.2)
178-178: Using
.strip()with multi-character strings is misleading(B005)
pyiron_atomistics/lammps/units.py (1)
19-47: LGTM! Well-defined conversion factors.The conversion factors are accurately defined using scipy.constants, ensuring precision and maintainability.
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
pyproject.toml (1)
45-47:⚠️ Potential issueVersion Attribute Issue
The dynamic version configuration attempts to readpyiron_lammps.__version__, yet the pipeline failure indicates that this attribute is missing. Please ensure that the__version__attribute is defined (for example, in the package’s__init__.pyor in a designated version module) so that Versioneer can correctly retrieve it.
🧹 Nitpick comments (26)
pyiron_lammps/output.py (7)
67-68: Prefer using direct membership check on dictionaries.Instead of
if "computes" in dump_dict.keys():preferif "computes" in dump_dict:to comply with Python best practices and avoid unnecessary method calls.- if "computes" in dump_dict.keys(): + if "computes" in dump_dict:🧰 Tools
🪛 Ruff (0.8.2)
67-67: Use
key in dictinstead ofkey in dict.keys()Remove
.keys()(SIM118)
94-95: Provide astacklevelargument for warnings.To improve tracebacks and help developers locate the source of warnings, supply
stacklevel=2(or higher) to indicate which line triggered the warning.- warnings.warn("LAMMPS warning: No log.lammps output file found.") + warnings.warn("LAMMPS warning: No log.lammps output file found.", stacklevel=2)🧰 Tools
🪛 Ruff (0.8.2)
94-94: No explicit
stacklevelkeyword argument found(B028)
161-177: Avoid usinglstripwith multi-character strings.Using
line.lstrip("ITEM: ATOMS")is potentially ambiguous. It strips any character in the provided string, not the entire substring in one sweep. Prefer a more explicit approach, e.g.:- columns = line.lstrip("ITEM: ATOMS").split() + columns = line.removeprefix("ITEM: ATOMS").split()🧰 Tools
🪛 Ruff (0.8.2)
176-176: Using
.strip()with multi-character strings is misleading(B005)
279-280: Remove redundant.keys()check.“
if kk not in dump.computes.keys():” can be replaced with “if kk not in dump.computes:”. This is more concise and Pythonic.- if kk not in dump.computes.keys(): + if kk not in dump.computes:🧰 Tools
🪛 Ruff (0.8.2)
279-279: Use
key not in dictinstead ofkey not in dict.keys()Remove
.keys()(SIM118)
326-327: Rename ambiguous loop variable "l".Variable names such as
lcan hinder readability. Consider renaming it toline_dataor similar to avoid confusion and meet style recommendations.🧰 Tools
🪛 Ruff (0.8.2)
326-326: Ambiguous variable name:
l(E741)
327-327: Ambiguous variable name:
l(E741)
413-414: Include astacklevelargument in warnings.When using
warnings.warn, specifyingstacklevelclarifies which caller triggered the warning, improving debuggability.- warnings.warn( + warnings.warn( "LAMMPS warning: log.lammps does not contain the required pressure values.", + stacklevel=2 )🧰 Tools
🪛 Ruff (0.8.2)
413-413: No explicit
stacklevelkeyword argument found(B028)
454-454: Addstacklevelfor runtime warning.Similarly, consider adding
stacklevel=2(or appropriate) for more accurate traceback.🧰 Tools
🪛 Ruff (0.8.2)
454-454: No explicit
stacklevelkeyword argument found(B028)
pyiron_lammps/structure.py (6)
114-115: Combine nestedifstatements.Use a single conditional with an
andto simplify flow. This is more readable and avoids excessive nesting.- if np.abs(d_a) < self.acc: - if d_a < 0: + if np.abs(d_a) < self.acc and d_a < 0: print("debug: apply shift") apre[1, 0] += 2 * d_a apre[2, 0] += 2 * d_a🧰 Tools
🪛 Ruff (0.8.2)
114-115: Use a single
ifstatement instead of nestedifstatementsCombine
ifstatements usingand(SIM102)
123-123: Usestacklevelinwarnings.warn.Adding a
stacklevelargument makes the warning more traceable.🧰 Tools
🪛 Ruff (0.8.2)
123-123: No explicit
stacklevelkeyword argument found(B028)
427-427: Rename unused loop variablei_shell.The variable
i_shellis unused. Rename it to_or remove it to avoid confusion and adhere to Pythonic style.🧰 Tools
🪛 Ruff (0.8.2)
427-427: Loop control variable
i_shellnot used within loop bodyRename unused
i_shellto_i_shell(B007)
491-492: Combine nestedifconditions into one.If
el_1_list is not Noneandlen(el_1_list) > 0, merge them. This ensures concise code.🧰 Tools
🪛 Ruff (0.8.2)
491-492: Use a single
ifstatement instead of nestedifstatements(SIM102)
529-536: Use ternary instead of if-else.For short assignments, a single-line expression can be clearer:
- if len(bond_type_lst) == 0: - num_bond_types = 0 - else: - num_bond_types = int(np.max(bond_type_lst)) + num_bond_types = 0 if len(bond_type_lst) == 0 else int(np.max(bond_type_lst))🧰 Tools
🪛 Ruff (0.8.2)
529-532: Use ternary operator
num_bond_types = 0 if len(bond_type_lst) == 0 else int(np.max(bond_type_lst))instead ofif-else-blockReplace
if-else-block withnum_bond_types = 0 if len(bond_type_lst) == 0 else int(np.max(bond_type_lst))(SIM108)
533-536: Use ternary operator
num_angle_types = 0 if len(angle_type_lst) == 0 else int(np.max(angle_type_lst))instead ofif-else-blockReplace
if-else-block withnum_angle_types = 0 if len(angle_type_lst) == 0 else int(np.max(angle_type_lst))(SIM108)
533-536: Apply the ternary operator forangle_type_lst.Same approach as with bond types:
- if len(angle_type_lst) == 0: - num_angle_types = 0 - else: - num_angle_types = int(np.max(angle_type_lst)) + num_angle_types = 0 if len(angle_type_lst) == 0 else int(np.max(angle_type_lst))🧰 Tools
🪛 Ruff (0.8.2)
533-536: Use ternary operator
num_angle_types = 0 if len(angle_type_lst) == 0 else int(np.max(angle_type_lst))instead ofif-else-blockReplace
if-else-block withnum_angle_types = 0 if len(angle_type_lst) == 0 else int(np.max(angle_type_lst))(SIM108)
pyiron_lammps/control.py (11)
236-237: Includestacklevelin warnings.When adjusting
n_printtomax_iter, usestacklevel=2for clarity in debugging.- warnings.warn("n_print larger than max_iter, adjusting to n_print=max_iter") + warnings.warn("n_print larger than max_iter, adjusting to n_print=max_iter", stacklevel=2)🧰 Tools
🪛 Ruff (0.8.2)
236-236: No explicit
stacklevelkeyword argument found(B028)
239-239: Use membership test without.keys().Change
if self["units"] not in LAMMPS_UNIT_CONVERSIONS.keys():toif self["units"] not in LAMMPS_UNIT_CONVERSIONS:.🧰 Tools
🪛 Ruff (0.8.2)
239-239: Use
key not in dictinstead ofkey not in dict.keys()Remove
.keys()(SIM118)
409-409: Remove redundant.keys().Replace
if self["units"] not in LAMMPS_UNIT_CONVERSIONS.keys():with a direct membership check.🧰 Tools
🪛 Ruff (0.8.2)
409-409: Use
key not in dictinstead ofkey not in dict.keys()Remove
.keys()(SIM118)
419-419: Use exception chaining.When re-raising an exception within an
exceptblock, attach the original error context for clarity:- raise NotImplementedError() + raise NotImplementedError() from None🧰 Tools
🪛 Ruff (0.8.2)
419-419: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
423-423: Addstacklevelfor the deprecation warning.Enhance traceability:
🧰 Tools
🪛 Ruff (0.8.2)
423-423: No explicit
stacklevelkeyword argument found(B028)
432-432: Likewise, specifystacklevel=2here.🧰 Tools
🪛 Ruff (0.8.2)
432-432: No explicit
stacklevelkeyword argument found(B028)
530-530: Set astacklevelinwarnings.warn.Clarify the call site for ignored Langevin usage.
🧰 Tools
🪛 Ruff (0.8.2)
530-530: No explicit
stacklevelkeyword argument found(B028)
631-631: Check membership with no.keys().Use
if self["units"] not in LAMMPS_UNIT_CONVERSIONS:instead of checking.keys().🧰 Tools
🪛 Ruff (0.8.2)
631-631: Use
key not in dictinstead ofkey not in dict.keys()Remove
.keys()(SIM118)
758-759: Add astacklevelto this warning.Helps users quickly identify the invocation site of the "replaced exponent" warning.
🧰 Tools
🪛 Ruff (0.8.2)
758-758: No explicit
stacklevelkeyword argument found(B028)
837-837: Providestacklevelfor warnings.Adding a
stacklevelis beneficial to trace the origin of this method-level notification.🧰 Tools
🪛 Ruff (0.8.2)
837-837: No explicit
stacklevelkeyword argument found(B028)
859-859: Specifystacklevelin the silent malfunction warning.Same reasoning: it improves developer awareness of the warning’s source.
🧰 Tools
🪛 Ruff (0.8.2)
859-859: No explicit
stacklevelkeyword argument found(B028)
pyiron_lammps/units.py (1)
231-240: Optimize dictionary lookup and warning usage.A few optimizations can be made to improve the code:
- Use
inoperator directly with dictionary- Add stacklevel to warning
- if label in quantity_dict.keys(): + if label in quantity_dict: return np.array( array * self.lammps_to_pyiron(quantity_dict[label]), dtype_dict[label] ) else: warnings.warn( message="Warning: Couldn't determine the LAMMPS to pyiron unit conversion type of quantity " - "{}. Returning un-normalized quantity".format(label) - ) + "{}. Returning un-normalized quantity".format(label), + stacklevel=2 + ) return array🧰 Tools
🪛 Ruff (0.8.2)
231-231: Use
key in dictinstead ofkey in dict.keys()Remove
.keys()(SIM118)
236-236: No explicit
stacklevelkeyword argument found(B028)
tests/test_control.py (1)
98-98: Useisoperator for None comparison.When comparing with None, use the
isoperator instead of==for identity comparison.- and tmp[3] == tmp[4] == tmp[5] == None + and tmp[3] is None and tmp[4] is None and tmp[5] is None- and tmp[3] == tmp[4] == tmp[5] == None + and tmp[3] is None and tmp[4] is None and tmp[5] is NoneAlso applies to: 103-103
🧰 Tools
🪛 Ruff (0.8.2)
98-98: Comparison to
Noneshould becond is NoneReplace with
cond is None(E711)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tests/static/potentials_lammps.csvis excluded by!**/*.csv
📒 Files selected for processing (31)
.ci_support/check.py(1 hunks).ci_support/environment-mpich.yml(0 hunks).ci_support/environment-old.yml(1 hunks).ci_support/environment-openmpi.yml(0 hunks).ci_support/environment.yml(1 hunks).coveralls.yml(0 hunks).github/workflows/black.yml(0 hunks).github/workflows/format_black.yml(0 hunks).github/workflows/pipeline.yml(1 hunks).github/workflows/pypicheck.yml(0 hunks).github/workflows/unittests-mpich.yml(0 hunks).github/workflows/unittests-old.yml(0 hunks).github/workflows/unittests-openmpi.yml(0 hunks).pre-commit-config.yaml(1 hunks)CITATION.cff(0 hunks)README.md(1 hunks)pyiron_lammps/__init__.py(0 hunks)pyiron_lammps/control.py(1 hunks)pyiron_lammps/output.py(1 hunks)pyiron_lammps/parallel.py(0 hunks)pyiron_lammps/structure.py(1 hunks)pyiron_lammps/units.py(1 hunks)pyproject.toml(2 hunks)tests/test_control.py(1 hunks)tests/test_elastic_base.py(0 hunks)tests/test_elastic_parallel_single_core.py(0 hunks)tests/test_elastic_parallel_two_cores.py(0 hunks)tests/test_error_messages.py(0 hunks)tests/test_evcurve_base.py(0 hunks)tests/test_evcurve_parallel_single_core.py(0 hunks)tests/test_evcurve_parallel_two_cores.py(0 hunks)
💤 Files with no reviewable changes (19)
- .github/workflows/black.yml
- .coveralls.yml
- .ci_support/environment-openmpi.yml
- .ci_support/environment-mpich.yml
- pyiron_lammps/init.py
- tests/test_error_messages.py
- tests/test_elastic_base.py
- tests/test_evcurve_base.py
- .github/workflows/format_black.yml
- .github/workflows/unittests-mpich.yml
- .github/workflows/pypicheck.yml
- tests/test_evcurve_parallel_two_cores.py
- tests/test_elastic_parallel_single_core.py
- .github/workflows/unittests-openmpi.yml
- tests/test_evcurve_parallel_single_core.py
- .github/workflows/unittests-old.yml
- tests/test_elastic_parallel_two_cores.py
- pyiron_lammps/parallel.py
- CITATION.cff
✅ Files skipped from review due to trivial changes (3)
- .ci_support/environment.yml
- README.md
- .pre-commit-config.yaml
🧰 Additional context used
🪛 GitHub Actions: Pipeline
.ci_support/check.py
[error] 9-9: Key 'optional-dependencies' does not exist in the TOML file.
pyproject.toml
[error] 1-1: Preparing metadata (pyproject.toml) did not run successfully. AttributeError: module 'pyiron_lammps' has no attribute 'version'.
🪛 Ruff (0.8.2)
pyiron_lammps/structure.py
114-115: Use a single if statement instead of nested if statements
Combine if statements using and
(SIM102)
123-123: No explicit stacklevel keyword argument found
(B028)
427-427: Loop control variable i_shell not used within loop body
Rename unused i_shell to _i_shell
(B007)
491-492: Use a single if statement instead of nested if statements
(SIM102)
529-532: Use ternary operator num_bond_types = 0 if len(bond_type_lst) == 0 else int(np.max(bond_type_lst)) instead of if-else-block
Replace if-else-block with num_bond_types = 0 if len(bond_type_lst) == 0 else int(np.max(bond_type_lst))
(SIM108)
533-536: Use ternary operator num_angle_types = 0 if len(angle_type_lst) == 0 else int(np.max(angle_type_lst)) instead of if-else-block
Replace if-else-block with num_angle_types = 0 if len(angle_type_lst) == 0 else int(np.max(angle_type_lst))
(SIM108)
pyiron_lammps/units.py
231-231: Use key in dict instead of key in dict.keys()
Remove .keys()
(SIM118)
236-236: No explicit stacklevel keyword argument found
(B028)
tests/test_control.py
98-98: Comparison to None should be cond is None
Replace with cond is None
(E711)
103-103: Comparison to None should be cond is None
Replace with cond is None
(E711)
pyiron_lammps/output.py
67-67: Use key in dict instead of key in dict.keys()
Remove .keys()
(SIM118)
94-94: No explicit stacklevel keyword argument found
(B028)
176-176: Using .strip() with multi-character strings is misleading
(B005)
279-279: Use key not in dict instead of key not in dict.keys()
Remove .keys()
(SIM118)
326-326: Ambiguous variable name: l
(E741)
327-327: Ambiguous variable name: l
(E741)
413-413: No explicit stacklevel keyword argument found
(B028)
454-454: No explicit stacklevel keyword argument found
(B028)
pyiron_lammps/control.py
236-236: No explicit stacklevel keyword argument found
(B028)
239-239: Use key not in dict instead of key not in dict.keys()
Remove .keys()
(SIM118)
409-409: Use key not in dict instead of key not in dict.keys()
Remove .keys()
(SIM118)
419-419: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
423-423: No explicit stacklevel keyword argument found
(B028)
432-432: No explicit stacklevel keyword argument found
(B028)
530-530: No explicit stacklevel keyword argument found
(B028)
631-631: Use key not in dict instead of key not in dict.keys()
Remove .keys()
(SIM118)
758-758: No explicit stacklevel keyword argument found
(B028)
837-837: No explicit stacklevel keyword argument found
(B028)
859-859: No explicit stacklevel keyword argument found
(B028)
🪛 YAMLlint (1.35.1)
.github/workflows/pipeline.yml
[warning] 191-191: wrong indentation: expected 4 but found 6
(indentation)
[error] 195-195: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (13)
pyiron_lammps/output.py (1)
122-126: Validate logic when checking for orthogonality.The
_check_ortho_prism()function returnsTrueif the prism is effectively orthorhombic. Raising aRuntimeErrorin this case could be confusing. Verify this logic to confirm that the correct scenario triggers the error..ci_support/environment-old.yml (1)
10-10:❓ Verification inconclusive
Verify scipy version.
The specified version
scipy =1.15.0appears to be incorrect as it's higher than the latest available version.
🌐 Web query:
What is the latest version of scipy?💡 Result:
The latest version of SciPy is 1.15.2, released on February 16, 2025[1][4]. This is a bug-fix release with no new features compared to version 1.15.1 but includes free-threaded Python 3.13 wheels for Linux ARM platforms[1][2].
Key details:
- Release type: Maintenance/bug-fix update
- Supported Python versions: 3.10-3.13[2]
- Key addition: First release with Python 3.13-compatible ARM Linux wheels[2]
Earlier versions in the 1.15.x series include:
- 1.15.1: Fixed critical
scipy.optimizeimport issues[2]- 1.15.0: Major release introducing sparse array improvements and new numerical integration tools[2]
The development team has shifted focus to maintaining the 1.15.x branch while adding new features to the main branch[2].
Citations:
- 1: https://scipy.org
- 2: https://github.com/scipy/scipy/releases
- 3: https://en.wikipedia.org/wiki/SciPy
- 4: https://pypi.org/project/scipy/
- 5: https://github.com/scipy/scipy
- 6: https://community.intel.com/t5/Intel-Distribution-for-Python/Request-newer-versions-of-numpy-amp-scipy/td-p/1437442
Action: Verify and Update the SciPy Version Specification
- The file
.ci_support/environment-old.ymlcurrently pins SciPy at version1.15.0.- According to the latest information, SciPy’s most recent release is
1.15.2(February 16, 2025).- Please confirm if the older version is intentional for compatibility or legacy reasons; otherwise, consider updating the dependency to
1.15.2.pyproject.toml (2)
2-2: Updated Build-System Dependencies
The[build-system]section now requires additional packages such ash5py,scipy,pyiron_base, andpyiron_snippets. Verify that these changes are aligned with the new file-based interface requirements.
28-33: Updated Project Dependencies
The dependencies list in the[project]section now includespyiron_base==0.11.5,pyiron_snippets==0.1.4,h5py==3.13.0,numpy==2.2.3, andscipy==1.15.2. Please confirm that these versions are compatible with the rest of the project components..github/workflows/pipeline.yml (9)
1-7: Pipeline Workflow Triggers
The workflow is triggered on pushes to themainbranch and on pull requests. This configuration appears sound for CI/CD needs.
8-17: Job "black" Configuration
The job for checking code formatting with Black is properly set up using the stable action. Verify that the source path (./${{ github.event.repository.name }}) correctly resolves to the repository content.
18-43: Job "black_fix" Configuration
The fallback job for formatting (triggered upon failure of the Black check) is correctly defined. The checkout with a token and use offetch-depth: 0are good practices for ensuring a successful commit push.
44-72: Job "coverage" Setup
The coverage job sets up the environment using Mambaforge and runs tests with coverage enabled. Confirm that the provided environment file (.ci_support/environment.yml) includes all necessary dependencies for accurate test execution.
73-94: Job "minimal" Configuration
The minimal test job correctly installs the project without extras and runs unit tests. This is helpful for quickly validating core functionality.
95-118: Job "pip_check" for Dependency Validation
The pip_check job runs a custom check script followed by a pip dependency verification. Ensure that the custom script (.ci_support/check.py) is updated to reflect the dependency changes in the project.
119-160: Job "unittest_matrix" – Cross-Platform Testing
The matrix strategy to run tests across different operating systems and Python versions is well implemented. Verify that the shared environment file (.ci_support/environment.yml) supports all target configurations.
161-182: Job "unittest_old" for Legacy Testing
This job verifies behavior on Python 3.9 using a separate environment file (.ci_support/environment-old.yml). Confirm that this file is kept in sync with any dependency changes from the project.
183-195: Job "autobot" for Automated Merging
The autobot job is configured to automatically merge PRs from Dependabot and pre-commit CI bots. Ensure that the conditional check and environment variables (e.g.,GH_TOKEN) are correctly configured for secure operation. Also, double-check that the indentation adheres to YAML standards (see specific comments below).🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 191-191: wrong indentation: expected 4 but found 6
(indentation)
[error] 195-195: no new line character at the end of file
(new-line-at-end-of-file)
.ci_support/check.py
Outdated
| lst = [] | ||
| for sub_lst in data["project"]["optional-dependencies"].values(): | ||
| for el in sub_lst: | ||
| lst.append(el) | ||
|
|
||
| data["project"]["dependencies"] += [ | ||
| el for el in set(lst) if not el.startswith("pwtools") | ||
| ] |
There was a problem hiding this comment.
Add error handling for missing 'optional-dependencies' key.
The script fails when the 'optional-dependencies' key doesn't exist in pyproject.toml. Add proper error handling and validation.
if __name__ == "__main__":
with open("pyproject.toml", "r") as f:
data = tomlkit.load(f)
+
+ # Validate TOML structure
+ if "project" not in data or "dependencies" not in data["project"]:
+ raise KeyError("Invalid pyproject.toml: missing 'project.dependencies' section")
+ # Create backup
+ with open("pyproject.toml.bak", "w") as f:
+ f.writelines(tomlkit.dumps(data))
+
lst = []
- for sub_lst in data["project"]["optional-dependencies"].values():
- for el in sub_lst:
- lst.append(el)
+ if "optional-dependencies" in data["project"]:
+ for sub_lst in data["project"]["optional-dependencies"].values():
+ for el in sub_lst:
+ lst.append(el)📝 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.
| lst = [] | |
| for sub_lst in data["project"]["optional-dependencies"].values(): | |
| for el in sub_lst: | |
| lst.append(el) | |
| data["project"]["dependencies"] += [ | |
| el for el in set(lst) if not el.startswith("pwtools") | |
| ] | |
| if __name__ == "__main__": | |
| with open("pyproject.toml", "r") as f: | |
| data = tomlkit.load(f) | |
| # Validate TOML structure | |
| if "project" not in data or "dependencies" not in data["project"]: | |
| raise KeyError("Invalid pyproject.toml: missing 'project.dependencies' section") | |
| # Create backup | |
| with open("pyproject.toml.bak", "w") as f: | |
| f.writelines(tomlkit.dumps(data)) | |
| lst = [] | |
| if "optional-dependencies" in data["project"]: | |
| for sub_lst in data["project"]["optional-dependencies"].values(): | |
| for el in sub_lst: | |
| lst.append(el) | |
| data["project"]["dependencies"] += [ | |
| el for el in set(lst) if not el.startswith("pwtools") | |
| ] |
🧰 Tools
🪛 GitHub Actions: Pipeline
[error] 9-9: Key 'optional-dependencies' does not exist in the TOML file.
| run: gh pr merge --auto --squash "$PR_URL" | ||
| env: | ||
| PR_URL: ${{github.event.pull_request.html_url}} | ||
| GH_TOKEN: ${{secrets.GITHUB_TOKEN}} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Append Newline at End of File
Ensure that the file ends with a newline character to satisfy YAML style guidelines and prevent linting errors.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 195-195: no new line character at the end of file
(new-line-at-end-of-file)
| runs-on: ubuntu-latest | ||
| if: (github.event.pull_request.user.login == 'dependabot[bot]' || github.event.pull_request.user.login == 'pre-commit-ci[bot]') | ||
| steps: | ||
| - name: Enable auto-merge for bot PRs |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Correct YAML Indentation
Static analysis detected that line 191 has an indentation issue (expected 4 spaces but found 6). Please adjust the indentation to conform to YAML linting guidelines. For example, change:
- - name: Enable auto-merge for bot PRs
+ - name: Enable auto-merge for bot PRs📝 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.
| - name: Enable auto-merge for bot PRs | |
| - name: Enable auto-merge for bot PRs |
🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 191-191: wrong indentation: expected 4 but found 6
(indentation)
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
Summary by CodeRabbit
New Features
CI/CD & Environment
Tests
Documentation