Skip to content

Conversation

@bruAristimunha
Copy link
Contributor

@bruAristimunha bruAristimunha commented Oct 11, 2025

PR Description

close #438.

I re-implemented the mne mechanism for the lock system. It was not possible to directly use the mne _open_lock, for two reasons:

  • first, it would make mne_bids compatible only with the latest version of mne, and
  • second, as we are writing more diversity of files compared to mne, tsv, for example, against .json of the config in mne, it was necessary to create a small routine to delete the lock files.

Merge checklist

Maintainer, please confirm the following before merging.
If applicable:

  • All comments are resolved
  • This is not your own PR
  • All CIs are happy
  • PR title starts with [MRG]
  • whats_new.rst is updated
  • New contributors have been added to CITATION.cff
  • PR description includes phrase "closes <#issue-number>"

@bruAristimunha
Copy link
Contributor Author

@sappelhoff, can you take a look when you have time, please?

@bruAristimunha bruAristimunha changed the title [MRG] Parallel writing of the object [WIP] Parallel writing of the object Oct 11, 2025
@bruAristimunha
Copy link
Contributor Author

bruAristimunha commented Oct 11, 2025

It is needed the other side, some test of writing, I think it will fail at the moment

@codecov
Copy link

codecov bot commented Oct 13, 2025

Codecov Report

❌ Patch coverage is 90.29957% with 68 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.91%. Comparing base (f61e0e0) to head (ebce415).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
mne_bids/_fileio.py 87.05% 22 Missing ⚠️
mne_bids/tests/test_write.py 59.57% 19 Missing ⚠️
mne_bids/tests/test_fileio.py 96.21% 11 Missing ⚠️
mne_bids/tests/test_read.py 89.55% 7 Missing ⚠️
mne_bids/write.py 92.68% 6 Missing ⚠️
mne_bids/tsv_handler.py 66.66% 2 Missing ⚠️
mne_bids/utils.py 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1451      +/-   ##
==========================================
- Coverage   97.41%   96.91%   -0.51%     
==========================================
  Files          41       43       +2     
  Lines        9371     9990     +619     
==========================================
+ Hits         9129     9682     +553     
- Misses        242      308      +66     

☔ 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.

Copy link
Member

@sappelhoff sappelhoff left a comment

Choose a reason for hiding this comment

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

@bruAristimunha the tests seem to pass now. Is this ready to be merged?

@bruAristimunha
Copy link
Contributor Author

not yet, i am creating more tests

@bruAristimunha
Copy link
Contributor Author

hey @agramfort,

Thank you for the revision 🙏🏽

Copy link
Member

@sappelhoff sappelhoff left a comment

Choose a reason for hiding this comment

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

@larsoner do you have time to have a final look at this?

@sappelhoff
Copy link
Member

image

Mhhh 🤔 the testing dataset should be available on the CI.

@bruAristimunha
Copy link
Contributor Author

I increase the cache because it was collapsing, let me reverse

@bruAristimunha bruAristimunha changed the title [WIP] Parallel writing of the object [MRG] Parallel writing of the object Oct 31, 2025
@bruAristimunha
Copy link
Contributor Author

@sappelhoff, I could only find the indication of this warning in the tests that didn't have the data, can you me for the test CI please?

@sappelhoff
Copy link
Member

no idea what happened there before but it seems all fixed now.

I think this can be merged once it got a final review by Eric.

Copy link
Member

@larsoner larsoner left a comment

Choose a reason for hiding this comment

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

Just a few minor tweaks, otherwise LGTM

@bruAristimunha
Copy link
Contributor Author

Many thanks @larsoner 🙏🏽

@larsoner larsoner merged commit b999865 into mne-tools:main Oct 31, 2025
22 of 24 checks passed
@larsoner
Copy link
Member

Thanks @bruAristimunha !

@larsoner
Copy link
Member

larsoner commented Nov 4, 2025

@bruAristimunha can you look into this failure:

https://app.circleci.com/pipelines/github/mne-tools/mne-nirs/3086/workflows/6f3e69ea-4467-4f0c-9072-08e0512d8134/jobs/3160

      File "/home/circleci/project/examples/general/plot_70_visualise_brain.py", line 287, in individual_analysis
        raw_intensity = read_raw_bids(bids_path=bids_path, verbose=False)
      File "<decorator-gen-512>", line 10, in read_raw_bids
      File "/home/circleci/python_env/lib/python3.10/site-packages/mne_bids/read.py", line 1120, in read_raw_bids
        raw = _handle_info_reading(sidecar_fname, raw)
      File "/home/circleci/python_env/lib/python3.10/site-packages/mne_bids/read.py", line 450, in _handle_info_reading
        with _open_lock(sidecar_fname, encoding="utf-8-sig") as fin:
      File "/usr/lib/python3.10/contextlib.py", line 135, in __enter__
        return next(self.gen)
      File "/home/circleci/python_env/lib/python3.10/site-packages/mne_bids/_fileio.py", line 155, in _open_lock
        _increment_lock_refcount(canonical_path)
      File "/home/circleci/python_env/lib/python3.10/site-packages/mne_bids/_fileio.py", line 196, in _increment_lock_refcount
        refcount_lock = filelock.FileLock(str(refcount_lock_path), timeout=5.0)
    AttributeError: 'bool' object has no attribute 'FileLock'

looks like filelock isn't installed (which should be okay) but in that case there is a failure...

@bruAristimunha
Copy link
Contributor Author

hey @larsoner,

should be fine with #1469

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.

Enabling parallelization of write_raw_bids

4 participants