-
Notifications
You must be signed in to change notification settings - Fork 102
Foof.group peak_params documentation doesn't mention additional column that is used as a index #192
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
Comments
Hey @danieltomasz - thanks for trying out the tool, and letting us know about unexpected behaviour! As you noticed, the As a quick note on the column being float - this is because the columns are stored together, so the [n_peaks, 4] array is type float. I'm not sure if there is a way to have mixed type arrays? In hindsight, a different strategy may have been to have multiple returns, the parameter array, and a integer column with indices (however, this gets a little messy since indices aren't needed for aperiodic parameters, which can also be returned from (I don't remember the exact details, but in a previous version, it might not have returned indices, but this would have made it impossible to map peaks -> spectra, once extracted). As it is, I think we should definitely add some details to the As a sidenote - depending what you are trying to do with the peaks, some of the stand-alone functions may help you process / organize the peaks: https://fooof-tools.github.io/fooof/api.html#analyze-periodic-components |
Thank you for a quick reply. I am transforming fooof group object results into a pandas data frame to visualize in seaborn distributions of some parameters across different datasets and conditions. Do utilities for such export exist? As one can observe, FOOOF is living in a world of arrays, not data frames :) Indeed,
In documentation, a short remark would be sufficient, FOOOFgroup is inheriting most of the documentation from simple FOOOF object, and there such problem doesn't exist |
Thanks for bringing this up. I'll add a note about the organization of results in the FOOOFGroup object. There currently isn't a utility to easily export results to a dataframe. I plan to explore an additional fit parameter that allows the results to be stored as a dictionary, so conversion to dataframe is easy: |
Also this specific fragment of code used to create additional column seems to be deprecated in latest numpy
|
Hey @danieltomasz - thanks for the note on the array issue. I had a look, and it turns out we didn't need to be typecasting to arrays in that way at all - this is fixed now in #197. In the same PR, I updated the Also, potentially of interest - we are currently exploring adding direct support for converting parameters to DataFrames, for example in #196. I think this issue is now addressed - if you have any suggestions related to the updates and/or works-in-progress, let me know! Otherwise, I think we can close this. |
Hi, I also think it is solved and addressed, I am closing this |
According to the Sphinx documentation on the fooof page (here and here) the code below should work
Code that doesn't work.
Documentation doesn't mention the additional column used as an index.
I was able to run it by changing it after reading this comment in code documentation
It was probably reworked in fooof 1.0 since my earlier code using the previous version doesn't have that problem (the 1st version worked). And why the index column is not an
int
?The text was updated successfully, but these errors were encountered: