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

Add neo objects as data sources #14

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

JuliaSprenger
Copy link

This PR adds functionality to use neo objects as sources of ephys data.

In the current implementation a specific structure of one neo.Block per subject needs to be provided, containing one neo.Segment for each frites epoch and holding exactly one neo.AnalogSignal with the data of that epoch.
Alternative ways of inputting these data would be a continuous AnalogSignal object containing all data of a subject accompanied by a neo.Epoch object that can be used for slicing the data. Currently this preprocessing step is left to the user.

Potentially frites could also make use of more neo features to extract, e.g. rois, but this would require a deeper understanding of the frites data concept on my side. @EtienneCmb If you think this might be interesting we should discuss this in more detail before implementing it.

This PR also introduces extra requirements in the setup to not introduce neo as a hard dependency.

@brovelli
Copy link
Member

Thanks Julia, great work

@EtienneCmb
Copy link
Collaborator

Hi Julia. Thank you for the PR. We should have discussed about the changes before you worked on it. I found an issue about the DatasetEphy that I first need to solve before Neo support. And its probably to change the form of the PR. I'll let you know once I found a solution.

@JuliaSprenger
Copy link
Author

@EtienneCmb No worry, you can first improve DatasetEphy and then we can adjust this PR. Any idea why the installation of the the neo does not seem to work for the CI? Is it caching the environment somehow?

@EtienneCmb
Copy link
Collaborator

I guess neo have to be installed also for github action (workflow)

@JuliaSprenger
Copy link
Author

@EtienneCmb Any progress on the restructuring of the DatasetEphy, so I can adjust this PR accordingly?

@EtienneCmb
Copy link
Collaborator

Hi @JuliaSprenger

I'm currently testing the new implementation on the develop branch. We should take a coffee and discuss about the best way to integrate neo inside Frites.

1 similar comment
@EtienneCmb
Copy link
Collaborator

Hi @JuliaSprenger

I'm currently testing the new implementation on the develop branch. We should take a coffee and discuss about the best way to integrate neo inside Frites.

@EtienneCmb EtienneCmb self-assigned this Nov 5, 2021
@JuliaSprenger
Copy link
Author

Hi @EtienneCmb I am not sure I understand your concept behind the sim_mi.py module. To me it looks like in many places the suj_ephy functionality could be used there for converting the data, but probably you had something else in mind.

@JuliaSprenger
Copy link
Author

Hi @EtienneCmb I am not sure I understand your concept behind the sim_mi.py module. To me it looks like in many places the suj_ephy functionality could be used there for converting the data, but probably you had something else in mind.

@EtienneCmb Any opinion on this?

@EtienneCmb
Copy link
Collaborator

Hi @JuliaSprenger, the sim_mi.py file contains functions to simulate data with statistical dependencies. This is mainly used for illustrating Frites' functions. You're right, it could use internally the suj_ephy to return an object "ready to use" in other functions. However, since not every users are familiar with xarray, I thought that returning standard NumPy arrays and then performing the conversion manually was probably clearer

@JuliaSprenger
Copy link
Author

@EtienneCmb So does it make sense to you to also add Neo support in the sim_mi module or is this rather minimal code for demonstration purposes that should not be cluttered with additional data format cases?

@EtienneCmb
Copy link
Collaborator

I would keep it simple, using NumPy. If you want to illustrate the functionalities with Neo, I would just add an example here.

For illustrating input types like MNE / NumPy / Xarray, I usually generate random data with NumPy and then convert them to MNE or Xarray for example. Could we do the same with neo?

@EtienneCmb
Copy link
Collaborator

Instead of having many :

try:
    import neo
    HAVE_NEO = True
except ModuleNotFoundError:
    HAVE_NEO = False

Could you make a is_neo_installed function in io_dependencies.py for testing if neo is installed?

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.

3 participants