MAINT: Modernize path handling in misc.py using pathlib#13775
MAINT: Modernize path handling in misc.py using pathlib#13775Akhila21-6 wants to merge 19 commits intomne-tools:mainfrom
Conversation
for more information, see https://pre-commit.ci
git status the commit.git push origin main
There was a problem hiding this comment.
Thanks for the PR!
FYI just to clarify, the issue you referenced is unrelated to this PR. This is because you referenced an issue in the wrong repo: The issue about modernizing path handling that you commented on the other day (mne-tools/mne-bids#1268 (comment) ) is in the MNE-BIDS satellite package, but this PR is for the MNE-Python package.
That being said, I'm not against migrating from os.path to pathlib in this module. I've left a few suggestions. Mostly, I think that the code comments are redundant because the code intent is already very clear.
Co-authored-by: Scott Huberty <52462026+scott-huberty@users.noreply.github.com>
Co-authored-by: Scott Huberty <52462026+scott-huberty@users.noreply.github.com>
Co-authored-by: Scott Huberty <52462026+scott-huberty@users.noreply.github.com>
Co-authored-by: Scott Huberty <52462026+scott-huberty@users.noreply.github.com>
Co-authored-by: Scott Huberty <52462026+scott-huberty@users.noreply.github.com>
|
Hi @scott-huberty, thank you for the catch! I've updated the PR
description to remove the incorrect reference to the MNE-BIDS issue. I have
also applied your suggestions to remove the redundant code comments and
clean up the extra line breaks. I appreciate the feedback and the
clarification on the repository structure!
…On Sun, 22 Mar 2026 at 21:07, Scott Huberty ***@***.***> wrote:
***@***.**** commented on this pull request.
Thanks for the PR!
FYI just to clarify, the issue you referenced is unrelated to this PR.
This is because the you referenced an issue in the wrong repo: The issue
about *modernizing path handling* that you commented on the other day (mne-tools/mne-bids#1268
(comment)
<mne-tools/mne-bids#1268 (comment)>
) is in the MNE-BIDS satellite package, but this PR is for the MNE-Python
package.
That being said, I'm not against migrating from os.path to pathlib in this
module. I've left a few suggestions. Mostly, I think that the code comments
are redundant because the code intent is already very clear.
------------------------------
In mne/misc.py
<#13775 (comment)>
:
> @@ -20,10 +22,13 @@ def parse_config(fname):
tmin, tmax, name, grad_reject, mag_reject,
eeg_reject, eog_reject
"""
+ # Convert fname to Path object to ensure compatibility
⬇️ Suggested change
- # Convert fname to Path object to ensure compatibility
------------------------------
In mne/misc.py
<#13775 (comment)>
:
> reject_params = read_reject_parameters(fname)
- with open(fname) as f:
- lines = f.readlines()
+ # Use pathlib to read text and split into lines directly
⬇️ Suggested change
- # Use pathlib to read text and split into lines directly
------------------------------
In mne/misc.py
<#13775 (comment)>
:
> @@ -37,6 +42,7 @@ def parse_config(fname):
break
else:
raise ValueError("Could not find event id.")
+
⬇️ Suggested change
-
------------------------------
In mne/misc.py
<#13775 (comment)>
:
> @@ -67,14 +73,20 @@ def read_reject_parameters(fname):
params : dict
The rejection parameters.
"""
- with open(fname) as f:
- lines = f.readlines()
+ # Ensure fname is a Path object
⬇️ Suggested change
- # Ensure fname is a Path object
------------------------------
In mne/misc.py
<#13775 (comment)>
:
> +
+ # Use pathlib to read lines
⬇️ Suggested change
-
- # Use pathlib to read lines
—
Reply to this email directly, view it on GitHub
<#13775?email_source=notifications&email_token=BLZNCGEYIEOAOGNY7X3UTNT4SACD3A5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTGOJYHAZDINRZGU3KM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#pullrequestreview-3988246956>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BLZNCGCJGIYJBYF7JBHW57T4SACD3AVCNFSM6AAAAACW2TIQKWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTSOBYGI2DMOJVGY>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
@Akhila21-6 1) Can you login to CircleCI with your Github Account? Also 2) please see our contributing guide for adding a changelog entry. After you do this then the remaining failing checks should run successfully. |
|
Hi @scott-huberty, I have authorized CircleCI with my GitHub account and added the changelog entry in doc/changes/devel/13775.maintenance.rst. Thank you for the guidance! |
| @@ -0,0 +1 @@ | |||
| Modernize path handling in ``mne/misc.py`` using ``pathlib``, by `Akhila Thammenenwar`_. | |||
There was a problem hiding this comment.
You added two changelog entries. The syntax in this file is correct but it should go in doc/changes/dev/.
Also please add your name and public profile to doc/changes/names.inc
for more information, see https://pre-commit.ci
|
Hi @scott-huberty, I have synced the branch, moved the changelog to the correct dev/ folder, and fixed the syntax for my entry in names.inc. All checks are now re-running. Thank you for your patience and guidance! |
| @@ -0,0 +1 @@ | |||
| Modernize path handling in ``mne/misc.py`` using ``pathlib``, by `Akhila Thammenenwar`_. | |||
There was a problem hiding this comment.
| Modernize path handling in ``mne/misc.py`` using ``pathlib``, by `Akhila Thammenenwar`_. | |
| Modernize path handling in ``mne/misc.py`` using ``pathlib``, by :newcontrib:`Akhila Thammenenwar`. |
|
@Akhila21-6 you seem to have deleted all other entries that were previously in |
Reference issue (if any)
This PR modernizes path handling in mne/misc.py using pathlib. (Note: A previous reference to #1268 was removed as it pertained to the mne-bids repository; however, this migration remains a beneficial standalone improvement for mne-python.)
What does this implement/fix?
This PR modernizes the file handling logic in mne/viz/misc.py by transitioning from traditional open() and os.path methods to the pathlib module.
Key Changes:
Modified parse_config and read_reject_parameters to use pathlib.Path.
Optimized file reading using Path.read_text().splitlines(), making the code more concise and Pythonic.
Ensured compatibility by wrapping fname in Path() to handle both string and path-like inputs.
Removed unused import os to keep the module clean.
Additional information
I am a GSoC 2026 applicant. I have verified these changes by running the following test suite locally on a Windows environment:
python -m pytest mne/viz/tests/test_misc.py
The tests resulted in 5 passed, 5 skipped (skips were due to missing large external datasets, but all relevant code logic tests passed). I successfully resolved local PATH and ModuleNotFoundError issues to ensure the environment was correctly configured for testing.