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

High level design plan: What belongs in which package? #57

Open
adam2392 opened this issue Nov 29, 2021 · 3 comments
Open

High level design plan: What belongs in which package? #57

adam2392 opened this issue Nov 29, 2021 · 3 comments

Comments

@adam2392
Copy link
Member

Summary

We had a chat with the Frites team and other independent developers on 11/29/21 to develop a robust Python suite of connectivity based analysis. To enable this, we agreed that on a high level that:

  • MNE-Connectivity should have mainly some "publication-tested" (say... sklearnish >300 citations?) connectivity measures, lower-level functionality such as the connectivity containers.
  • and Frites should have research-geared (and any possibly more complex ones) connectivity measures and higher level functionality, such as statistical analysis and single subject/group level analysis. The statistical analysis could be ported into MNE-connectivity if they need to operate on container levels? TBD.

This then should keep in line w/ the resources dedicated to both projects: mne-connectivity can move faster then mne-python and Frites can move faster then both.

PR Plan

Here, we should enumerate the PRs necessary to bring this to completion once we both review the code in frites and mne-connectivity.

Lmk If I missed anything.

cc:
@brovelli
@adam2392
@EtienneCmb
@ViniciusLima94
@brovelli
@jshanna100

@adam2392 adam2392 added this to To do in Collaboration with Frites via automation Nov 29, 2021
@adam2392
Copy link
Member Author

TODO list for MNE-Connectivity

  1. a PR to the docs page pointing to Frites
  2. a PR to enable exporting of all Connectivity data structures to data frame (http://xarray.pydata.org/en/stable/generated/xarray.DataArray.to_dataframe.html). Probably just need to add this function wrapper to _Connectivity base class, or just add an example demonstrating it.
  3. [ ]

more to come...

@EtienneCmb
Copy link

Hi @adam2392,

Thanks for starting the discussion.

From our side we could add a function conn_frites_to_mne for converting to your base objects. Something like that : conn_frites_to_mne.

I could also add a input parameter to_mne=True/False to each frites.conn function for returning the outputs into MNE format. Something like :

# conversion to mne-connectivity base object
if to_mne:
    return conn_frites_to_mne(conn)

What do you think?

@adam2392
Copy link
Member Author

adam2392 commented Dec 1, 2021

Container Discussion

^ tagging this as a container discussion.

What do you think?

Thanks! Overall this is definitely in the right direction I think. See below for specific thoughts.

From our side we could add a function conn_frites_to_mne for converting to your base objects.
I could also add a input parameter to_mne=True/False to each frites.conn function for returning the outputs into MNE format. Something like :

I think my only concern here is the maintainability of such a standard. Say the connectivity containers are evolving and allow additional functionality, a PR would be needed to each frites.conn function to update them accordingly. It could also be missed. A way to counteract this is of course sufficient unit-tests that are versioned w/ the current main branch of mne-connectivity.

proposal for container consolidation

I was looking over the Subject and Group containers in frites. I am wondering for the subject-level containers, is there any difference between that and any of the containers in mne? I'm wondering if there's a way we can just consolidate the containers in one package, so we don't need the if/else cases? Of course, then we can add GroupLevel containers on mne side.

The only missing data points I can see are the "subject name", "agg_ch" and "multivariate" parameters in https://brainets.github.io/frites/api/generated/frites.dataset.SubjectEphy.html. Not 100% sure of their functionality though. If they're easy enough to port, this can then simplify the frites codebase?

Then, for example https://brainets.github.io/frites/api/generated/frites.conn.conn_dfc.html can return either the xarray, or 1 connectivity container.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants