@coderabbitai full review

Walkthrough

The visualization API was refactored to a stateful, chainable setup/draw pattern: SpaceRenderer now stores portrayals and draw kwargs via setup_* methods and executes drawings without per-call kwargs. Space drawer parameter names were standardized (space/ chart kwargs renamed), and Solara integration now calls draw_* without forwarding kwargs.

Changes

Sequence Diagram(s)

sequenceDiagram
    participant Page as Solara Viz
    participant Renderer as SpaceRenderer
    participant Drawer as Space Drawer

    Note over Page,Renderer: New stateful, chainable flow
    Page->>Renderer: setup_structure(**draw_space_kwargs)
    Renderer-->>Page: self
    Page->>Renderer: setup_agents(agent_portrayal, **draw_agent_kwargs)
    Renderer-->>Page: self
    Page->>Renderer: setup_propertylayer(propertylayer_portrayal)
    Renderer-->>Page: self
    Page->>Renderer: render()
    Note right of Renderer: Uses stored state
    Renderer->>Drawer: draw_matplotlib() / draw_altair()
    Drawer-->>Renderer: rendered output
    Renderer-->>Page: visualization

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Attention areas:
  • Verify all call sites updated to use setup_* and no longer pass portrayals/kwargs to draw/render.
  • Confirm consistent initialization and use of draw_space_kwargs / draw_agent_kwargs across matplotlib and altair paths.
  • Review deprecation-warning logic and dict→callable conversion for propertylayer portrayals.
  • Ensure examples and tests fully reflect the new API surface.

Possibly related PRs

• Updating Examples and Docs inline with the new API. #2819 — touches SpaceRenderer usage and example updates; likely overlaps in API changes for portrayals.
• Introduction of Space Renderer #2803 — similar refactor of SolaraViz and SpaceRenderer to use stateful setup/draw calls; strong code-level relation.

Suggested labels

enhancement, example

Suggested reviewers

• quaquel
• tpike3
• Corvince

Poem

🐰 With chains of setup, I hop and store,

Draw kwargs tucked behind my door.
No more passing each time you call,
I remember portraits, one and all.
A tiny hop—render once for all. ✨

Pre-merge checks and 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.

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

I made this kind of in a hurry. I’ll update the tests and add the documentation over the weekend. In the meantime, can you take a look at just the API to see if it’s working fine (it was for me)?

1. Do we want to keep backward compatibility? I am fine with breaking things, but it's good to discuss this

We did just release an API update over on a 3.x version, updating it again in 3.3.x wouldn't be very wise in my opinion.

2. All examples still need to be updated. Do you plan to make that part of this PR?

I intend to, what do you suggest?

We did just release an API update over on a 3.x version, updating it again in 3.3.x wouldn't be very wise in my opinion.

Fair enough, but then we should ensure that we don't break it. So, e.g., draw_agent should become something like the following

def draw_agents(self, *args, **kwargs):
   if args or kwargs: # implicit booleanness of empty list/kwarg to be checked if correct
       warnings.warn("some message", DeprecationWarning, stack_level=2)
       self.agent_portrayal = args[0]
       self_draw_kwargs = kwargs
   # normal code goes here.

I intend to, what do you suggest?

Excellent, fixing it in this pr makes most sense to me.

1. You are using a PendingDerpecationWarning. I am not sure that is the best one to use. See the discussion on stack overflow, I personally favor using a FutureWarning (for all users of a library) these days, or alternatively aDeprecationWarning (for code that uses mesa from within __main__).

the discussion also mentions:

PendingDeprecationWarning means "you're going to have to change something eventually", and FutureWarning means "something in the way you're using this isn't correct, and may lead to failure later."

and the term FutureWarning is not descriptive as to what it's referring to.
As @EwoutH mentioned, there will be a minor release before fully deprecating this API, so using DeprecationWarning is also not an option.

2. There are a few other changes here (e.g., the name change of kwargs in space_drawers, which technically were not needed and might cause trouble for users. Curious to know why you made the change. I am inclined to suggest reverting those changes.

I just wanted to make naming consistent in the backend, this is not user facing, so I don't think we should worry about that. Let me know if you feel otherwise.

3. Have you tested all examples, or are you only relying on the unit tests for this? I just want to ensure that all examples continue to work correctly after this PR is merged.

I've tested all the examples by running them myself, they will work after this PR is merged if I haven't missed anything.

I will try to write up some guidelines on how we deprecate stuff. Than we don’t have to discuss and reinvent that on every PR.

Ok, fine with me. Let's first agree on deprecation in #2900 and then afterwards merge this.

Could you check if we now follow the policy as described in #2900?

From what I can tell this follows #2900 (step 1). @Sahil-Chhoker and @quaquel is this ready to merge?

just a last check for the deprecations and then yes it's ready to go.

@quaquel your formal review status is still “requested changes”, blocking merging.

@Sahil-Chhoker ready to merge, right?

Thanks Jan!

@Sahil-Chhoker, we're good?

@EwoutH, CI seems to be failing.