-
Notifications
You must be signed in to change notification settings - Fork 1k
Allowing color mapping to integers in AgentPortrayalStyle #2818
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Allowing color mapping to integers in AgentPortrayalStyle #2818
Conversation
@coderabbitai full review |
✅ Actions performedFull review triggered. |
WalkthroughThe changes expand the accepted types for the Changes
Estimated code review effort1 (~5 minutes) Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
mesa/visualization/components/portrayal_components.py (3)
49-49
: Expose a reusable alias & inline comment for new numeric color supportThe literal union is getting long and is repeated elsewhere in the codebase. Introduce a
ColorLike
alias (close to Matplotlib’s semantics) and add a short in-line comment so downstream maintainers immediately understand the intent.- color: str | tuple | int | float | None = "tab:blue" + # Accepts any Matplotlib-compatible color: name, RGB(A) tuple, + # or scalar (int/float) consumed by a colormap. + ColorLike = str | tuple | int | float + color: ColorLike | None = "tab:blue"
18-45
: Sync the docstring with the new scalar-color capabilityThe descriptive block and example still mention only strings/tuples. Add one sentence and a tiny code snippet showing
color=agent.wealth
(scalar mapped throughcmap
). Prevents confusion for users scanning the docs.
49-55
: Fail-fast validation for scalar colorsNow that bare numbers are accepted, negative values (or very large ones) will raise cryptic errors deep in Matplotlib. A quick
__post_init__
guard such as:if isinstance(self.color, (int, float)) and self.color < 0: raise ValueError("Scalar color values must be non-negative")would surface problems earlier without noticeable overhead.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
mesa/visualization/backends/matplotlib_backend.py
(0 hunks)mesa/visualization/components/portrayal_components.py
(1 hunks)
💤 Files with no reviewable changes (1)
- mesa/visualization/backends/matplotlib_backend.py
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: run-benchmarks
Performance benchmarks:
|
@Sahil-Chhoker looks good for the code rabbit review any concerns integrating its feedback on
|
Summary
Allowing color mapping to integers in AgentPortrayalStyle
Motive
This could only be achieved in the
dict
version ofagent_portrayal
, but also now possible inAgentPortrayalStyle
.Implementation
Main problem was the collect_agent_data function in matplotlib used to assume the color of edgecolors as the defined color if edgecolors are not defined, this removes this assumption and allows the mapping of color with a proper colormap.
Usage Examples
Boltzman wealth model works with this:
Summary by CodeRabbit
Bug Fixes
New Features