Skip to content

FIX: correct label orientation for 0-90 deg nodes in plot_connectivity_circle#13855

Open
paavalipopov wants to merge 10 commits intomne-tools:mainfrom
paavalipopov:fix-label-orientation-circle
Open

FIX: correct label orientation for 0-90 deg nodes in plot_connectivity_circle#13855
paavalipopov wants to merge 10 commits intomne-tools:mainfrom
paavalipopov:fix-label-orientation-circle

Conversation

@paavalipopov
Copy link
Copy Markdown

Reference issue (if any)

What does this implement/fix?

In mne.viz.circle._plot_connectivity_circle, the condition on line 341 if angle_deg >= 270 that determines labels horizontal alignment only covers the 270–360° range (3–6 o'clock). Nodes in the 0–90° range (12–3 o'clock) are incorrectly treated as left-half nodes, and their labels appear pointing inward, which looks like a bug.

Fix: extend the condition to angle_deg >= 270 or angle_deg < 90, matching the full right half of the circle.

Additional information

Here's a visual comparison of connectograms generated using the original ("old") and patched ("new") versions of mne-tools.

comparison

@paavalipopov paavalipopov requested a review from drammock as a code owner April 20, 2026 22:31
@welcome
Copy link
Copy Markdown

welcome Bot commented Apr 20, 2026

Hello! 👋 Thanks for opening your first pull request here! ❤️ We will try to get back to you soon. 🚴

Copy link
Copy Markdown
Contributor

@CarinaFo CarinaFo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me, only a minor change in the test docs and the location of the test function (just updated)

Comment thread mne/viz/tests/test_connectivity_circle.py Outdated
paavalipopov added a commit to paavalipopov/mne-python that referenced this pull request Apr 20, 2026
Comment thread mne/viz/tests/test_connectivity_circle.py Outdated
@CarinaFo
Copy link
Copy Markdown
Contributor

CarinaFo commented Apr 21, 2026

It looks like this branch includes several unrelated commits or working-tree changes. Could you reset/rebase the branch onto current main and recommit just the relevant files? Based on the commit history, three commits are unrelated to the current work

@paavalipopov paavalipopov force-pushed the fix-label-orientation-circle branch from 7e49ef8 to f96ce9f Compare April 21, 2026 20:24
@scott-huberty
Copy link
Copy Markdown
Contributor

Hey @paavalipopov Can you please create a CircleCI account by signing into CircleCI via GitHub? After that you can push a commit here, and the documentation build job should run.

Copy link
Copy Markdown
Contributor

@CarinaFo CarinaFo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you and congrats to your first PR, looks good to me.

@paavalipopov
Copy link
Copy Markdown
Author

Thanks for the review and the help with the cleanup, @CarinaFo! Glad to contribute to mne tools.

Copy link
Copy Markdown
Member

@drammock drammock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @paavalipopov this is a nice PR. Thanks for the thorough test. I have a couple of suggestions to tidy it a bit, which I think should be uncontroversial so I'll apply them and mark for merge.

Comment thread mne/viz/tests/test_circle.py Outdated
Comment thread mne/viz/tests/test_circle.py Outdated
Comment thread mne/viz/tests/test_circle.py Outdated
Co-authored-by: Daniel McCloy <dan@mccloy.info>
@drammock drammock enabled auto-merge (squash) April 23, 2026 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants