fix: correct compress() density docstring to kg/m³#124
Merged
Conversation
The compress() docstring documented `density` as g/cm^3, but the function passes it to calculate_box_dimensions(), which expects kg/m^3 (consistent with pack() and calculate_density()). A caller trusting the docstring would pass e.g. 1.0 for water and get a box 1000x too sparse. The runtime logic was already correct; this corrects only the misleading docstring, standardizing on the package-wide kg/m^3 convention. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe ChangesDocstring Unit Correction
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~1 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
molify.compress's docstring documented itsdensityargument as g/cm³, but the function passes that value straight tocalculate_box_dimensions, which expects kg/m³ — the same convention already used bypackandcalculate_density. A user trusting the docstring would pass1.0for water (1 g/cm³) and get a box 1000× too sparse.The runtime logic was already correct and consistent with the rest of the package; only the docstring was wrong. This standardizes
compresson the package-wide kg/m³ convention by fixing the one misleading line.Verification (TDD)
The existing
test_compress/test_compress_freezealready pin the kg/m³ contract. To confirm they genuinely guard it, I temporarily applied the naivedensity * 1000"fix":calculate_density→ ~1,000,000 instead of 1000).This demonstrates the kg/m³ contract is real and tested, so the docstring correction now matches verified behavior.
Test Plan
uv run pytest→ 454 passed×1000conversion breaks the suite (RED), proving the contract is guarded🤖 Generated with Claude Code
Summary by CodeRabbit