search

Eden-Kramer-Lab / spectral_connectivity

Frequency domain estimation and functional and directed connectivity analysis tools for electrophysiological data
GNU General Public License v3.0
121 stars 43 forks source link

Adds Dataset with multiple connectivity methods and skips unneeded attributes #18

Closed mmyros closed 3 years ago

mmyros commented 3 years ago

See wrapper.multitaper_connectivities()

mmyros commented 3 years ago

wrapper.multitaper_connectivities() is an alternative to wrapper.multitaper_connectivity() that is potentially more useful as it can accept multiple methods and return them all in the same set of coordinates

coveralls commented 3 years ago

Coverage Status

Coverage increased (+1.0%) to 81.967% when pulling 41e1503686ecdc48201e54a8647cb6f00024ff38 on mmyros:master into dbd64858fd63c4e8a9c86895fbe416a073b6bd21 on Eden-Kramer-Lab:master.

edeno commented 3 years ago

Is there a way this can all just be one function multitaper_connectivity? Like you could pass either a string or a iterable as the method (aka a list of strings) and this would be handled properly inside of the one function. What do you think?

mmyros commented 3 years ago

Yes, I wanted to get your opinion on how to best approach this. There will be a difference in the output depending on whether or the "method" is a string or iterable: output will be DataArray or Dataset. DataArray can be plotted right away as xar.plot(), whereas Dataset needs to have das["method"].plot(). This will be true even is 'method' is an iterable length one. Do you think that's ok?

edeno commented 3 years ago

Ah I see what you mean. I guess the options are:

  1. Separate function
  2. Make it a DataArray if there is only one method and a Dataset if there are multiple methods (within the function)
  3. Always have it be a Dataset

I feel like 3 will provide the most consistent interface for the user even if there isn't the magic of just being able to plot immediately, although I do admit that 2 is also pretty nice and users would probably figure it out. 1 is my least favorite because I think the best part of having the combined function is just quick ease of use and two functions seems confusing.

I'm okay with either 2 or 3. 3 will probably lead to the least prolonged confusion because the user will always use the same syntax if they understand the example (although not guaranteed). Do you have a preference for one over the other?

mmyros commented 3 years ago

Having thought about this a bit more, I implemented option 2. It may be a bit tricky to document, but it feels like the convenience is worth it. Also, sticking with a string input may be easiest for a complete newcomer to coding. Feel free to override me though!