Add AbsValue, Modulus, and Power math components#29
Add AbsValue, Modulus, and Power math components#29gerardl wants to merge 2 commits intobbartling:developfrom
Conversation
|
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)
📝 WalkthroughWalkthroughAdded three kitControl Math components (AbsValue, Modulus, Power): updated README checklist, added Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/bog_builder/models.py (1)
171-173: Make new math input slot types explicit inSLOT_TYPE_MAPPING.Right now these inputs rely on fallback typing. Defining them explicitly keeps the type system deterministic and self-documenting.
♻️ Suggested update
SLOT_TYPE_MAPPING = { + ("kitControl:AbsValue", "inA"): "StatusNumeric", + ("kitControl:Modulus", "inA"): "StatusNumeric", + ("kitControl:Modulus", "inB"): "StatusNumeric", + ("kitControl:Power", "inA"): "StatusNumeric", + ("kitControl:Power", "inB"): "StatusNumeric", ("kitControl:MultiVibrator", "period"): "RelTime", ... }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bog_builder/models.py` around lines 171 - 173, SLOT_TYPE_MAPPING currently lacks explicit entries for the new math node input slots (e.g., "kitControl:AbsValue" inputs "inA"; "kitControl:Modulus" inputs "inA","inB"; "kitControl:Power" inputs "inA","inB"), causing them to fall back to implicit typing; update SLOT_TYPE_MAPPING to add explicit slot type mappings for these nodes (map each "inA" and "inB" to the appropriate numeric slot type used elsewhere in SLOT_TYPE_MAPPING) so the type system is deterministic and self-documenting, referencing the node keys "kitControl:AbsValue", "kitControl:Modulus", and "kitControl:Power" when adding the entries.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Around line 531-533: The API reference table in README is missing entries for
the newly implemented math wrappers; update the methods table to include
add_abs_value, add_modulus, and add_power (matching the checklist just above) by
adding rows describing each method name, signature, brief description, and any
relevant params/return types so the README’s method table and checklist are
consistent; locate the methods table near the current math section and insert
the three entries with identical formatting to existing rows for other math
wrappers.
---
Nitpick comments:
In `@src/bog_builder/models.py`:
- Around line 171-173: SLOT_TYPE_MAPPING currently lacks explicit entries for
the new math node input slots (e.g., "kitControl:AbsValue" inputs "inA";
"kitControl:Modulus" inputs "inA","inB"; "kitControl:Power" inputs "inA","inB"),
causing them to fall back to implicit typing; update SLOT_TYPE_MAPPING to add
explicit slot type mappings for these nodes (map each "inA" and "inB" to the
appropriate numeric slot type used elsewhere in SLOT_TYPE_MAPPING) so the type
system is deterministic and self-documenting, referencing the node keys
"kitControl:AbsValue", "kitControl:Modulus", and "kitControl:Power" when adding
the entries.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3c938cd6-9f53-4fe3-87a1-93c703f11900
📒 Files selected for processing (3)
README.mdsrc/bog_builder/builder.pysrc/bog_builder/models.py
Hello, very cool project! I've been working with Niagara a lot lately and was hoping something like this existed. I'm happy to build out more of the kitControl components but just did a few to make sure that was okay with you. I can also start adding tests if that would be helpful.
Summary by CodeRabbit
New Features
Documentation