Skip to content

FORWARD: clarify or implement reference-field path in _lead_dots (currently NotImplementedError) #13755

Description

@PragnyaKhandelwal

Describe the new feature or enhancement

While reviewing forward internals, I found an unimplemented branch in mne/forward/_lead_dots.py:

if rref is not None:
    raise NotImplementedError  # we don't ever use this, isn't tested

I propose clarifying this behavior by either implementing this reference-field path with tests, or explicitly documenting/raising a clearer unsupported-mode error.

Describe your proposed implementation

Preferred approach:

  • confirm expected behavior with maintainers (support vs explicitly unsupported)
  • if supported: implement the rref branch and add targeted tests
  • if unsupported: replace the generic NotImplementedError with an actionable error message and add a short docs/changelog note

Describe possible alternatives

  1. Leave as-is
  • simplest, but keeps an ambiguous untested failure path
  1. Implement immediately without alignment
  • faster, but may conflict with intended forward-model design

Chosen approach (align first, then implement or explicitly mark unsupported) reduces risk.

Additional context

I checked related forward/report activity (#2828, PR #13722), which appears focused on report-level sensitivity-map integration, not this low-level _lead_dots branch. So this is intended as separate forward-core tracking.

Activity

  1. PragnyaKhandelwal commented on Mar 15, 2026

    @PragnyaKhandelwal
    ContributorAuthor

    Could maintainers confirm whether the rref is not None path in _lead_dots.py should be implemented and tested, or explicitly kept unsupported? I can prepare a focused PR either way.

  2. larsoner commented on Mar 16, 2026

    @larsoner
    Member

    I think based on how we call the code, we should be guaranteed never to hit a case where rref is None. In modern code we'd just do assert rref is None # guaranteed by our code or similar nowadays I think

  3. PragnyaKhandelwal commented on Mar 17, 2026

    @PragnyaKhandelwal
    ContributorAuthor

    Thanks, that makes sense. I implemented the invariant check as suggested (assert rref is None) in _lead_dots.py, and validated with focused forward tests.

    I’ll open a small PR linked to this issue.

  4. PragnyaKhandelwal commented on Mar 17, 2026

    @PragnyaKhandelwal
    ContributorAuthor

    Merged in #13764. Closing this now!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions