Add Windows specific dump options

[zm711]

Fixes #2187 and #1891.

When in doubt follow Sam's intuition. I just do a platform.system check and if windows I don't use relative_to. Now it lets me sort from (I've tested this on 3 separate data sets now--but if you want me to do some more specific local tests let me know):

c->d
c->network

with no problems. I know @h-mayorquin would prefer not to have OS specific behavior, but this works. So for Windows users that do want to sort and switch drives this seems like the best solution for now (maybe we add a place holder test for the future when testing occurs on all os's?)

I also added this to pickle, but can delete if you only want this on the json dump.

Unfortunately to use run_sorter Windows users must use mapped drives and not unc paths because the cmd prompt where the spikeinterface script is run does not accept uncs paths. So this doesn't fix #2186 (unfortunately--since for some parts of SI we need unc paths and for others we need mapped paths).

[zm711]

Did we want to add a deprecation warning in the else in this PR?

[h-mayorquin]

@zm711 Sam is fine with doing windows testing it seems. I was against having different OS behavior without testing because neither me Alessio or Sam use Windows. We would get errors that we would not be able to catch or test. But this should be a moot point once we do windows testing in the CI. Probably mid-December seeing how much work we have : /

[h-mayorquin]

You are absically throwing the relative_to out in windows.

Wouldn't it better to instead tell users that this option is not supported in windows so they don't run away with the impression that they did indeed save their files with relative paths?

[zm711]

So add a warning? Or an assert level and make them re-enter without relative_to?

EDIT: another question. Isn't relative_to internal only so how would a Windows users know? Other functions have an argument like relative_path which requires that the code be on the same drive/same folder in Linux. So I'm not sure what relative_to actually gains the user on Windows?

[zm711]

I'm still reading up on this part of the code. I just implemented Sam's suggestion, but haven't read too deep into the rest of the function. Once I made his change I tested and it worked, but I agree Windows users should know. For instance if I do c-> c I could do relative_to without problem. So maybe there is a more universal change for Windows that would be better.

[h-mayorquin]

Is that a folder that you are saving or a file?
Are you able to load it again?

[zm711]

Both. So I have tested this with run_sorter and run_sorter_by_property with Kilosort2 and Kilosort3 (I am able to read the folder of files with read_kilosort and with Mountainsort5 (saves one .npz file, which I am able to read with read_npz_sorting). I haven't tested any sort of portability yet though. But the provenance files/log files all give absolute paths, so I'm assuming they are not portable, but I can test Monday.

[h-mayorquin]

Yes, they are not portable. Can you load them with the base_folder keyword, this is what I mean,

[zm711]

Could you give me a code snippet of what you want me to try please? I have never loaded anything with the base_folder keyword.

[zm711]

So testing
recording.dump_to_pickle(..)
recording.dump(..)

both failed with this stack trace

  File ~\Documents\GitHub\spikeinterface\src\spikeinterface\core\base.py:600 in dump
    self.dump_to_json(file_path, relative_to=relative_to, folder_metadata=folder_metadata)

  File ~\Documents\GitHub\spikeinterface\src\spikeinterface\core\base.py:633 in dump_to_json
    dump_dict = self.to_dict(

  File ~\Documents\GitHub\spikeinterface\src\spikeinterface\core\base.py:437 in to_dict
    dump_dict = _make_paths_relative(dump_dict, relative_to)

  File ~\Documents\GitHub\spikeinterface\src\spikeinterface\core\base.py:1014 in _make_paths_relative
    return recursive_path_modifier(d, func, target="path", copy=True)

  File ~\Documents\GitHub\spikeinterface\src\spikeinterface\core\core_tools.py:824 in recursive_path_modifier
    recursive_path_modifier(kwargs, func, copy=False)

  File ~\Documents\GitHub\spikeinterface\src\spikeinterface\core\core_tools.py:847 in recursive_path_modifier
    dc[k] = func(v)

  File ~\Documents\GitHub\spikeinterface\src\spikeinterface\core\base.py:1013 in <lambda>
    func = lambda p: os.path.relpath(str(p), start=relative)

  File ~\anaconda3\envs\spykes\Lib\ntpath.py:754 in relpath
    raise ValueError("path is on mount %r, start on mount %r" % (

ValueError: path is on mount 'C:', start on mount 'D:'

trying sorting.dump_to_pickle failed with:

  File ~\Documents\GitHub\spikeinterface\src\spikeinterface\core\base.py:659 in dump_to_pickle
    file_path.write_bytes(pickle.dumps(dump_dict))

PicklingError: Can't pickle <function BaseExtractor.from_dict at 0x000001C297972020>: it's not the same object as spikeinterface.core.base.BaseExtractor.from_dict

So it seems relative_to fails on Windows when moving between drives as a feature. Is there something else I can test? Or do you think this would need to be a deeper change then just at the sorting level in this case?

[samuelgarcia]

Hi Zach,
sorry for the delay to give feedbakc on this.
Here you are removing the relative machanism for windows in sorter.

We discuss a bit and finally we will implement sometings in the same direction :

• at all place we use relative_to dump
• add a function that check if relative_to is possible or not.
• if not possible then use the absolute if possible then use the relative.

This function checking could be : I am on windows + path are unc or dos one + path are on same drive

I propose to start this soon.

This would close this PR so. I leave this PR open as a reminder (to avoid procastination).

[zm711]

Sounds good all! Thanks.