Fix `NeuralynxRawIO` reading in one-dir mode with nested directories #1777

h-mayorquin · 2025-09-18T18:24:27Z

Users sometimes place extra directories and files under the Neuralynx root. Today, we exclude by extension, but in “one-dir” mode the reader asserts that every entry must be a file, which raises an error:

python-neo/neo/rawio/neuralynxrawio/neuralynxrawio.py

Lines 217 to 231 in 72ec76a

    
           if self.rawmode == "one-dir": 
        
               filenames = sorted(os.listdir(self.dirname)) 
        
           else: 
        
               filenames = self.include_filenames 
        
           filenames = [f for f in filenames if f not in self.exclude_filenames] 
        
           full_filenames = [os.path.join(self.dirname, f) for f in filenames] 
        
           for filename in full_filenames: 
        
               if not os.path.isfile(filename): 
        
                   raise ValueError( 
        
                       f"Provided Filename is not a file: " 
        
                       f"{filename}. If you want to provide a " 
        
                       f"directory use the `dirname` keyword" 
        
                   )

We should fix this to (1) make the workflow more forgiving for users and (2) align behavior with include_filenames, which already allows nested folders.

Proposed change: restructure file discovery into three steps.

Gather candidates by mode:
- multiple-files: validate existence only for the explicitly provided filenames.
- one-dir: list directory entries and keep only regular files via Path.is_file() (silently ignore subdirectories).
Apply the excluded-filenames filter.
Keep only paths with valid Neuralynx extensions.

This ensures directories are ignored in “one-dir” mode while preserving strict validation in “multiple-files” mode, and it removes redundant extension checks from the main processing loop.

cc @weiglszonja

h-mayorquin · 2025-09-18T18:39:29Z

neo/rawio/neuralynxrawio/neuralynxrawio.py

        if filename is not None:
            include_filenames = [filename]
-            warnings.warn("`filename` is deprecated and will be removed. Please use `include_filenames` instead")
+            warnings.warn("`filename` is deprecated and will be removed in v1.0. Please use `include_filenames` instead")


From here:
#1440 (comment)

zm711 · 2025-09-19T21:26:40Z

@PeterNSteinmetz, we are trying to keep you in the loop. We are planning on making some adjustments in gap detection across our RawIOs (see #1773), which means we are working on various IOs in anticipation of our more general BaseRawIO level changes. If you have time we would love you to check out this PR and #1779. If you don't have time we can move forward but we appreciate all your help so if you have discussion points as we make these changes feel free to let us know.

PeterNSteinmetz · 2025-09-19T21:33:23Z

I think this change, #1777, will cause it to throw a ValueError is a directory name is provided in include_filenames. It will not look for files in sub-directories.

h-mayorquin · 2025-09-19T21:43:46Z

I think this change, #1777, will cause it to throw a ValueError is a directory name is provided in include_filenames. It will not look for files in sub-directories.

I don't understand, the current behavior on master is that to throw an error in both "one-dir" and "multiple-files" when there is a nested folder:

python-neo/neo/rawio/neuralynxrawio/neuralynxrawio.py

Lines 217 to 232 in 72ec76a

    
           if self.rawmode == "one-dir": 
        
               filenames = sorted(os.listdir(self.dirname)) 
        
           else: 
        
               filenames = self.include_filenames 
        
           filenames = [f for f in filenames if f not in self.exclude_filenames] 
        
           full_filenames = [os.path.join(self.dirname, f) for f in filenames] 
        
           for filename in full_filenames: 
        
               if not os.path.isfile(filename): 
        
                   raise ValueError( 
        
                       f"Provided Filename is not a file: " 
        
                       f"{filename}. If you want to provide a " 
        
                       f"directory use the `dirname` keyword" 
        
                   )

PeterNSteinmetz · 2025-09-19T21:48:59Z

Yes, I guess it throws an error with sub-directories and one-dir mode. Sorry I thought you were quoting the proposed fix above. I see in the commit list the changes. Those appear that they should work. Is there a test added for this?

h-mayorquin · 2025-09-19T21:53:43Z

Yes, I guess it throws an error with sub-directories and one-dir mode. Sorry I thought you were quoting the proposed fix above. I see in the commit list the changes. Those appear that they should work. Is there a test added for this?

Ah, that makes sense.

Regarding the test,
Yes, I added a test to ensure that random nested directories do not throw the reader off.

zm711

a couple very cosmetic things.

neo/rawio/neuralynxrawio/neuralynxrawio.py

zm711 · 2025-09-23T12:24:44Z

neo/rawio/neuralynxrawio/neuralynxrawio.py

            _, ext = os.path.splitext(filename)
-            ext = ext[1:]  # remove dot
-            ext = ext.lower()  # make lower case for comparisons
-            if ext not in self.extensions:
-                continue
+            ext = ext[1:].lower()  # remove dot and make lower case


You were converting the other part to Pathlib. Why not here? Not that it is required all in one PR. Just curious.

I did not want the PR to grow too big.

I am happy to do a full change to pathlib in another PR if you think is worth it.

It's not necessary for me so I would say let's just put that on the back burner.

Co-authored-by: Zach McKenzie <[email protected]>

zm711 · 2025-09-23T21:42:52Z

neo/rawio/neuralynxrawio/neuralynxrawio.py

 import pathlib
+from pathlib import Path


Why the double import? if we need pathlib couldn't you just use pathlib.Path below? Although I vaguely remember you telling me that imports from the standard library are basically free.

I just did not see the other and reverted to my habit of importing like that.

zm711 · 2025-09-23T21:44:04Z

neo/rawio/neuralynxrawio/neuralynxrawio.py

+        else:  # one-dir mode
+            # For one-dir mode, get all files from directory
+            dir_path = Path(self.dirname)
+            file_paths = [item for item in sorted(dir_path.iterdir()) if item.is_file()]


And just for my own learning is there any risk of doing sorted here? If different os's lead to different sortings could that lead to some non-deterministic behavior.

This was the previous behavior. I think that it will be more uniform if we sort rather that if we just collect directly. That is, is more deterministic with sorting not less.

I only bring this up because I think I tried something like this on Mac vs Windows and found that for some file type they had a different behavior. So per OS it is deterministic, but if we make assumption about order across OS it might not be true. But I don't remember and I don't have a citation.

I see, that's surprising to me. The order is just based on the file name so I guess if the are full paths the slashes might change the thing?

Anyway, I think we should just preserve the old behavior and order by file name.

Fixed. Now it should be exactly as it was before.

fix one-dir mode in neuralynx

e30e104

h-mayorquin changed the title ~~Fix one-dir mode in NeuralynxRawIO with nested dicts~~ Fix NeuralynxRawIO reading in one-dir mode with nested directories Sep 18, 2025

h-mayorquin commented Sep 18, 2025

View reviewed changes

h-mayorquin marked this pull request as ready for review September 19, 2025 14:48

zm711 reviewed Sep 23, 2025

View reviewed changes

h-mayorquin and others added 2 commits September 23, 2025 11:18

Update neo/rawio/neuralynxrawio/neuralynxrawio.py

e0269c2

Co-authored-by: Zach McKenzie <[email protected]>

Update neo/rawio/neuralynxrawio/neuralynxrawio.py

d9c96b0

Co-authored-by: Zach McKenzie <[email protected]>

zm711 reviewed Sep 23, 2025

View reviewed changes

h-mayorquin added 3 commits September 23, 2025 16:11

sort by name and not by file path

6834457

pathlib

74deb34

Merge branch 'master' into neuralynx_fix_one_dir_nested_directory

4823f12

zm711 merged commit ab3d0f5 into NeuralEnsemble:master Sep 24, 2025
3 checks passed

h-mayorquin deleted the neuralynx_fix_one_dir_nested_directory branch September 24, 2025 17:30

zm711 added this to the 0.14.3 milestone Oct 4, 2025

	if self.rawmode == "one-dir":
	filenames = sorted(os.listdir(self.dirname))
	else:
	filenames = self.include_filenames

	filenames = [f for f in filenames if f not in self.exclude_filenames]
	full_filenames = [os.path.join(self.dirname, f) for f in filenames]

	for filename in full_filenames:
	if not os.path.isfile(filename):
	raise ValueError(
	f"Provided Filename is not a file: "
	f"{filename}. If you want to provide a "
	f"directory use the `dirname` keyword"
	)

Fix NeuralynxRawIO reading in one-dir mode with nested directories #1777

Fix NeuralynxRawIO reading in one-dir mode with nested directories #1777

Uh oh!

Conversation

h-mayorquin commented Sep 18, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zm711 commented Sep 19, 2025

Uh oh!

PeterNSteinmetz commented Sep 19, 2025

Uh oh!

h-mayorquin commented Sep 19, 2025

Uh oh!

PeterNSteinmetz commented Sep 19, 2025

Uh oh!

h-mayorquin commented Sep 19, 2025

Uh oh!

zm711 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

h-mayorquin Sep 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Fix `NeuralynxRawIO` reading in one-dir mode with nested directories #1777

Fix `NeuralynxRawIO` reading in one-dir mode with nested directories #1777

h-mayorquin Sep 23, 2025 •

edited

Loading