Add unit tests for planner.py

[iliasmahboub]

Closes #122.

Adds 13 unit tests covering print_plane() and the plan class.
Heavy dependencies (brainrender, vedo) are mocked so tests run in
~2.5s without GPU or atlas download. planner.py coverage: 0% -> 100%.

Tests cover: plane output formatting and rounding, region list-to-dict
conversion, 3D format enforcement, arrow scale defaults, slicer plane
printing, kwarg forwarding to Heatmap, show() return value and root
mesh alpha.

How to test

pytest tests/test_unit/test_planner.py -v

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.42%. Comparing base (d02e7a9) to head (da337a6).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #133   +/-   ##
=======================================
  Coverage   87.42%   87.42%           
=======================================
  Files           5        5           
  Lines         326      326           
=======================================
  Hits          285      285           
  Misses         41       41           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[alessandrofelder]

Thanks a lot for your contribution @iliasmahboub
I've managed to have a first look and have raised a few questions. I like the range of things you test in this PR.

On a general level: I'm not sure the complexity added by the mocking (which makes the test code harder to read) is worth the improvement in test speed (which IIUC is your rationale?) but I am open to being convinced otherwise. Apologies if I misunderstood!

[alessandrofelder]

Should the return value of mock_init not be set to None already in line 72, without you having to set it manually in line 75? Same in other test functions. But maybe I am misunderstanding the code?

[alessandrofelder]

Could we not use an actual Plane object here, so we don't have to remember to change this code if we change the attributes of Plane (e.g. hypothetically we rename self.u to self.vector_u or something)?

[alessandrofelder]

Why do we mock print_plane here? It doesn't seem to be used?

[iliasmahboub]

@alessandrofelder Hi Alessandro! apologies for the slow reply; I've had final exams and project deadlines this past week and only just caught this in my inbox. Thank you for the detailed review! You're right on all three points:

• The redundant return_value = None assignments are unnecessary since it's already set in the decorator. I'll remove them.
• FakePlane is a liability if Plane's interface changes; I'll swap in real Plane objects.
• The unused print_plane mock was a copy-paste artifact and I'll drop it.
  On the broader mocking question: the intent was to avoid triggering the actual Heatmap initialization, which depends on atlas data, rather than purely a speed concern. That said, I'm happy to restructure the tests to use real objects if that fits better with how the rest of the suite is written.

I'll push a fix shortly! Have a nice day!

[iliasmahboub]

@alessandrofelder also, to elaborate, I mentioned the 2.5s runtime in the PR description which was a side effect, not the main reason. My main line of thought was isolation: Heatmap.init creates a real brainrender.Scene and brainrender.Atlas, loads atlas metadata, adds meshes, and instantiates a Slicer. Since plan.init calls super().init() directly, running these tests unmocked without a local atlas cache fails before any planner logic runs. The mock keeps this as a unit test of the planner's own behavior, independent of atlas availability.
The fair caveats that it doesn't cover the end to end path with real atlas data, which belongs in an integration test. But for testing the planner's argument handling in isolation, I think the mock is pretty justified imo. Happy to remove it if you think otherwise though!

[iliasmahboub]

@alessandrofelder I pushed a follow-up commit addressing the planner test review comments: removed the redundant init return_value assignments, replaced the FakePlane test helper with real Plane instances, and removed unused print_plane patch decorators where the mock was not asserted.\n\nI also reran the targeted planner tests and ruff locally: pytest tests\test_unit\test_planner.py -q --no-cov, and ruff check tests\test_unit\test_planner.py.