Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fs paramater specification causing problem in load_physio #10

Open
me-pic opened this issue Oct 28, 2024 · 5 comments · May be fixed by #11
Open

fs paramater specification causing problem in load_physio #10

me-pic opened this issue Oct 28, 2024 · 5 comments · May be fixed by #11
Assignees
Labels

Comments

@me-pic
Copy link
Contributor

me-pic commented Oct 28, 2024

Expected Behavior

In the generate_physio function, when load_physio is called, it uses the fs specified in the parameters of generate_physio. However, for '.phys' files, this causes problem since the fs information can be retrieved from the file directly. So having fs=None (default value) lead to a fs value of NOTHING (enum type) after loading the file...

See also discussion here.

Actual Behavior

Steps to Reproduce the Problem

1.
2.
3.

Specifications

- Python version:
- peakdet version:
- Platform:

Possible solution

Since the fs parameters should still be specified if loading any file format that is not .phys, I would propose to create another mode to separate those different format rather than grouping everything under mode="physio"

@me-pic
Copy link
Contributor Author

me-pic commented Oct 28, 2024

@smoia wdyt ?

@smoia
Copy link
Member

smoia commented Oct 28, 2024

I didn't have a look at the code yet, is it possible that passing arguments more carefully will solve the problem?

Also, yes, fs should be in a 'phys' file, but they should also be in the json file in BIDS format, and I don't remember us supporting anything else than that?

Worth adding a check in the affected function rather than duplicating code, but maybe you have another solution in mind I'm not 100% understanding.

@me-pic
Copy link
Contributor Author

me-pic commented Oct 28, 2024

Should we then constrain the possible file extension here to only .phys here ?

@smoia
Copy link
Member

smoia commented Oct 28, 2024

Ah I see. Then yes, please separate the .physio from the other formats, creating another I/O function for those. In those, we can make sure that fs is given while loading somehow.
Unrelated, here's some code for input of those formats:

    if isinstance(data, str):
        if os.path.exists(data):
            ext = os.path.splitext(data)[-1]

            if ext == '.gz':
                ext == os.path.splitext(os.path.splitext(data)[0])[-1]
                    
            if ext == '.tsv':
                data = np.genfromtxt(data, delimiter='\t')
            elif ext == '.csv':
                data = np.genfromtxt(data, delimiter=',')
            else:
                data = np.genfromtxt(data)
        else:
            raise IOError(f'Cannot find {data}')

You can also use reading functions form phys2bids on top of that!

@me-pic
Copy link
Contributor Author

me-pic commented Oct 29, 2024

Thank you for that block of code, will integrate that !

@me-pic me-pic linked a pull request Oct 29, 2024 that will close this issue
18 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants