Skip to content

Conversation

@dodu94
Copy link
Member

@dodu94 dodu94 commented Dec 18, 2025

quick fix for new behaviour of F4Enix with the introduction of the Nuclide object

Summary by CodeRabbit

Release Notes

  • Chores

    • Updated f4enix dependency to version 0.18.0 or higher.
  • Refactor

    • Enhanced reaction object creation logic to align with updated dependencies.

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

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 18, 2025

Walkthrough

Updated f4enix dependency from >= 0.15.0 to >= 0.18.0, and modified input processing logic to construct Nuclide objects using Nuclide.from_int_string instead of plain strings when handling non-Material ZAID values.

Changes

Cohort / File(s) Summary
Dependency Update
pyproject.toml
Bumped f4enix minimum version from 0.15.0 to 0.18.0
Nuclide Object Construction
src/jade/run/input.py
Added import for Nuclide from f4enix.input.irradiation; modified InputD1SSphere to use Nuclide.from_int_string() for constructing Reaction objects with ZAID and daughter parameters instead of plain string arguments

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • Verify that Nuclide.from_int_string() is the intended API in f4enix >= 0.18.0
  • Confirm that the Reaction constructor accepts Nuclide instances for both ZAID and daughter parameters
  • Check that the change doesn't break downstream ReactionFile processing

Poem

🐰 With nuclides transformed by string and by number,
Our code springs to life from the upgrade's sweet slumber,
From 0.15 to 0.18 we bound,
Where better APIs and clean construction are found! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'fix new f4enix behaviour' is vague and generic, using non-descriptive terms that do not clearly convey what specific issue is being fixed or which behavioral change is being addressed. Consider a more specific title that identifies the actual problem being fixed, such as 'Update code to use Nuclide objects from f4enix >= 0.18.0' or 'Fix InputD1SSphere to handle new f4enix Nuclide API'.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ 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 fix-f4enix-v0.18-compatibility

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7ba1fd9 and 36f50a7.

📒 Files selected for processing (2)
  • pyproject.toml (1 hunks)
  • src/jade/run/input.py (2 hunks)
⏰ 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). (12)
  • GitHub Check: test (windows-latest, 3.10, false)
  • GitHub Check: test (ubuntu-latest, 3.12, false)
  • GitHub Check: test (ubuntu-latest, 3.10, false)
  • GitHub Check: test (windows-latest, 3.11, false)
  • GitHub Check: test (windows-latest, 3.12, false)
  • GitHub Check: test (ubuntu-latest, 3.11, false)
  • GitHub Check: test (windows-latest, 3.10, false)
  • GitHub Check: test (ubuntu-latest, 3.10, false)
  • GitHub Check: test (windows-latest, 3.12, false)
  • GitHub Check: test (windows-latest, 3.11, false)
  • GitHub Check: test (ubuntu-latest, 3.11, false)
  • GitHub Check: test (ubuntu-latest, 3.12, false)
🔇 Additional comments (3)
src/jade/run/input.py (2)

15-15: LGTM - Import necessary for Nuclide object support.

The import is correctly added to support the new f4enix 0.18.0 behavior where ZAID values are represented as Nuclide objects.


463-467: Verify the format f"{zaid}.{lib.suffix}" for Nuclide.from_int_string() with f4enix library documentation.

The code constructs nuclide identifiers in the standard MCNP format (e.g., "92235.31c"), which aligns with nuclear library conventions. However, verify this format is explicitly supported by from_int_string() in the f4enix version being used. Additionally, error handling is absent—consider wrapping the Nuclide.from_int_string() calls in try-except to gracefully handle invalid inputs.

pyproject.toml (1)

25-25: Verify version 0.18.0 stability and review breaking changes between 0.15.0 and 0.18.0.

The dependency is being updated from >= 0.15.0 to >= 0.18.0 (3 minor version jump). The code uses f4enix APIs including Nuclide.from_int_string(), Reaction, and ReactionFile for D1S irradiation handling (lines 450-468 in src/jade/run/input.py). Ensure that:

  1. Version 0.18.0 is officially stable and released
  2. Review f4enix release notes for any breaking changes that could affect this or other parts of the codebase
  3. Confirm the Nuclide.from_int_string() method and related classes have compatible APIs between versions

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

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

@dodu94
Copy link
Member Author

dodu94 commented Dec 18, 2025

FYI @alexvalentine94 , this is why the tests are failing again, it is a quick fix due to an update of F4Enix version, no need for a real review

@dodu94 dodu94 merged commit cfba320 into developing Dec 18, 2025
11 of 13 checks passed
@dodu94 dodu94 deleted the fix-f4enix-v0.18-compatibility branch December 18, 2025 16:26
@codecov
Copy link

codecov bot commented Dec 18, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
src/jade/run/input.py 88.01% <100.00%> (+0.04%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

2 participants