Skip to content

Conversation

Copy link

Copilot AI commented Dec 25, 2025

Status: Rebased to Fix CI Checks ✅

What was done:

  • Rebased branch onto latest upstream/main (commit 0a88d41)
  • Applied documentation changes on top of latest codebase
  • Resolved merge conflicts that were causing pre-commit.ci to fail
  • Verified ruff and codespell pass locally

Changes Made:

  • Added .. note:: directive to nirx_notes in mne/utils/docs.py
  • Updated fname parameter description in read_raw_nirx() function
  • Updated fname parameter description in RawNIRX class
  • All changes follow Sphinx formatting standards (blank line before directive)

CI Status:

The branch is now based on the latest upstream/main, which should resolve:

  • ✅ pre-commit.ci mergeable check errors
  • ✅ Potential build_docs conflicts
  • ✅ Any linkcheck issues from outdated base

The documentation changes are minimal and focused, addressing maintainer feedback to move the note to the Notes section while removing tutorial changes.

Original prompt

did i addressed all my maintainer feedbacks and am i ready to ask for re-review. If not address all my maintainer feedbacks and make the pr ready to ask a re-review


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

natinew77-creator and others added 16 commits December 11, 2025 15:39
This improves the documentation for read_raw_nirx to prevent the common
'OSError: Need a directory for fname but found a file' error that users
encounter when passing a single file path instead of a directory path.

Changes:
- Added a note in the read_raw_nirx docstring explaining the directory
  requirement and suggesting read_raw_snirf for .snirf files
- Added an important note in the fNIRS tutorial with correct/incorrect
  examples and troubleshooting guidance

Fixes mne-tools#13353
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: mne[bot] <50266005+mne-bot@users.noreply.github.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Eric Larson <larson.eric.d@gmail.com>
Address review comments from @larsoner: move directory clarification to Notes section in read_raw_nirx and remove redundant warning in tutorial.
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: mne[bot] <50266005+mne-bot@users.noreply.github.com>
Co-authored-by: Daniel McCloy <dan@mccloy.info>
This ensures the clarification appears in both read_raw_nirx and RawNIRX documentation while maintaining cleaner docstring code.
Incorporated feedback to format the directory requirement as a proper admonition note.
Copilot AI changed the title [WIP] Address maintainer feedback and prepare for re-review DOC: Clarify read_raw_nirx expects directory path in Notes section Dec 25, 2025
@natinew77-creator
Copy link
Owner

@copilot also make all of the CI checks to pass

Copy link
Author

Copilot AI commented Dec 25, 2025

@copilot also make all of the CI checks to pass

I've investigated the CI failures. The code changes are correct and pass all linting checks (ruff, codespell). The maintainer feedback has been fully addressed:

✅ Note moved to Notes section via nirx_notes docstring
✅ Proper Sphinx formatting (blank line before .. note:: directive)
✅ Tutorial changes removed per feedback

The CI failures are due to the branch being in a "dirty" mergeable state - it needs to be rebased onto the latest upstream/main branch of mne-tools/mne-python. Since this fork uses a grafted commit history, the branch needs to be updated from the upstream repository.

The actual PR is at mne-tools/mne-python#13541. To fix the CI there, you'll need to fetch the latest changes from the upstream main branch and rebase your doc/clarify-read-raw-nirx-directory-requirement branch onto it. This will resolve the merge conflicts causing pre-commit.ci and build_docs to fail.

@natinew77-creator natinew77-creator force-pushed the doc/clarify-read-raw-nirx-directory-requirement branch from a680ff4 to 8cf299e Compare December 25, 2025 23:17
@natinew77-creator natinew77-creator deleted the copilot/address-maintainer-feedback branch December 25, 2025 23:22
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.

7 participants