Skip to content

Convert path_input to str in _process_CLI_arguments - #4188

Open
barlowa124 wants to merge 1 commit into
openmc-dev:developfrom
barlowa124:init-lib-pathlike-4185
Open

barlowa124 wants to merge 1 commit into
openmc-dev:developfrom
barlowa124:init-lib-pathlike-4185

Conversation

@barlowa124

@barlowa124 barlowa124 commented Oct 11, 2026 •

Copy link
Copy Markdown

Description

Model.init_lib(directory=Path(...)) fails with AttributeError: 'PosixPath' object has no attribute 'encode' (#4185). The directory argument is documented as PathLike, is passed through as path_input to _process_CLI_arguments, and reaches openmc.lib.init, which calls .encode() on every argument. restart_file is converted with str() in the same function. path_input was appended unchanged.

The fix converts path_input with str(), matching the restart_file handling two blocks earlier. The subprocess-based callers (run, calculate_volumes) accept str arguments, so they are unaffected.

plot_geometry takes a path_input documented as str only and appends it unchanged. That path is left alone since subprocess.Popen accepts os.PathLike arguments and the annotation does not promise PathLike support.

str() was used rather than os.fspath() to match the restart_file conversion directly above it. For a PathLike argument the two are equivalent.

No documentation change is needed. init_lib already annotates directory as PathLike | None, so the change brings the implementation in line with the documented contract.

tests/unit_tests had no coverage of _process_CLI_arguments.

The new test_executor.py checks that path_input comes back as str for both str and Path inputs. It could not be run locally because import openmc requires a built libopenmc and this environment has no HDF5 toolchain for the C++ build. The function was verified standalone instead. Before the change it returned a PosixPath for a Path input. After the change it returns str.

Fixes #4185

AI Assistance

  • Harness: Devin CLI
  • Model: SWE-2 Max
  • Reasoning effort: not applicable

Checklist

  • I have performed a self-review of my own code
  • I have run clang-format (version 18) on any C++ source files (if applicable)
  • I have followed the style guidelines for Python source files (if applicable)
  • I have made corresponding changes to the documentation (if applicable)
  • I have added tests that prove my fix is effective or that my feature works (if applicable)

Summary by CodeRabbit

  • Bug Fixes

    • Path-like inputs are now accepted when building command-line arguments, with paths consistently passed as strings.
  • Tests

    • Added coverage for string and path-like inputs.

The returned args are documented as a list of str, but path_input was
appended unchanged. openmc.lib.init() calls .encode() on every argument,
so Model.init_lib(directory=Path(...)) raised AttributeError after
exporting the XML files. subprocess-based callers are unaffected.

Fixes: openmc-dev#4185
@coderabbitai

coderabbitai Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 6e134791-4fc0-4c35-8a76-2f9157966647

📥 Commits

Reviewing files that changed from the base of the PR and between aa4afc4 and 45c7249.


📒 Files selected for processing (2)
  • openmc/executor.py
  • tests/unit_tests/test_executor.py

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.



📝 Walkthrough

Walkthrough

The CLI argument builder now converts the input path to a string before appending it. Tests check string and Path inputs and verify that all returned arguments are strings.

Changes

CLI path argument handling

Layer / File(s) Summary
Stringify and test path input
openmc/executor.py, tests/unit_tests/test_executor.py
_process_CLI_arguments converts path_input to a string. Tests cover string and Path inputs and check that every returned argument is a string.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~5 minutes

Change: Bug fix · Severity of issue fixed: Medium


Merge Risk: ⚪ Minimal · up to 45c72

The path conversion change is covered by focused tests, with no identified material risk remaining before merge.

Pre-merge checks | Passed 6 | Failed 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (6 passed)
Check name Status Explanation
Description check Passed The description explains the bug, root cause, fix, testing status, issue reference, AI assistance, and checklist state.
Linked Issues check Passed The change directly addresses issue #4185 by allowing pathlib.Path values in Model.init_lib(directory=...).
Out of Scope Changes check Passed The implementation and tests are focused on the reported path conversion bug. No unrelated changes are described.
Title check Passed The title is concise, specific, and accurately describes the primary code change.
Linked Issues check Passed Issue #4185 requires Model.init_lib(directory=pathlib.Path(...)) to accept the directory and initialize from its XML files. The pull request converts path_input to str in `_process_CLI_arguments…
Out of Scope Changes check Passed The changes are limited to the path_input conversion in openmc/executor.py and focused unit tests for that behavior. Both changes directly support issue #4185. No unrelated production or test chan…

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Model.init_lib raises AttributeError when directory is a pathlib.Path

1 participant