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

[WIP] Flexible dtype results class #194

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

[WIP] Flexible dtype results class #194

wants to merge 4 commits into from

Conversation

ryanhammonds
Copy link
Contributor

@ryanhammonds ryanhammonds commented Feb 13, 2021

This addresses #192 and a point in #193. This would allow the datatype of fm.peak_params_ to be converted to different datatypes (OrderedDict and pd.DataFrame), similar to how df.to_dict() works in pandas (note: this only returns the new datatype and doesn't change the results class datatype). I have only implemented the results class object, but this design would eventually allow fm.peak_params_.to_dict() or fm.peak_params_.to_pandas() to work. Here is an example usage of the current changes:

import numpy as np
from fooof.data import FitParams

peak_params = np.array([[1, 2, 3], [4, 5, 6]])
labels = ['CF', 'PW', 'BW']

pe_results = FitParams(peak_params, labels)

# Methods of FitParams are inherited from np.ndarray
pe_results * 10

# Return results as a dataframe
pe_pandas = pe_results.to_pandas()

# Return results as a OrderedDict
pe_dict = pe_results.to_dict()

@fooof-tools fooof-tools deleted a comment from codecov-io Feb 13, 2021
@ryanhammonds
Copy link
Contributor Author

@TomDonoghue, do you think this is a good idea to pursue further?

@fooof-tools fooof-tools deleted a comment from codecov-io Feb 23, 2021
@ryanhammonds
Copy link
Contributor Author

ryanhammonds commented Feb 23, 2021

This has been extended so fm.get_results().to_pandas() and fm.get_results().to_dict() works. It looks a bit nicer as:

results = fm.get_results()
df = results.to_pandas()

This maintains backwards compatibility so fm.get_results() still returns a NamedTuple - now with added methods to convert to a dataframe or OrderedDict.

@TomDonoghue
Copy link
Member

TomDonoghue commented Feb 23, 2021

Thanks for working on these ideas here @ryanhammonds - and sorry I'm slow to respond here.

I have a feeling that this approach might not be the best way to go. In part, tests currently fail because the fit functions need to flexible in the number of params they take (including zero for periodic fit), and so pulling the labels from the functions is an issue (also: peak params aren't quite the same quantities as the gaussian fit, for example).

Also, I think we need to focus on how this works at the group level - as that's the main use case (exporting a DF for a single model isn't that likely). At the group level, the param are stored a bit differently. Also, this is where the complication arises, since there needs to be some organization of peak parameters (what to do if model 0 has 1 peak, and model 1 has 2 peaks, etc).

I think what probably makes the most sense is a helper function that converts a FOOOFResults object to a pandas Series (one row of a DF), with options to control the peak parameters), and then at the group level this can be done for each model results and concatenated together.

EDIT: I've played around with a possible alternative approach in #196

@ryanhammonds
Copy link
Contributor Author

ryanhammonds commented Feb 23, 2021

The tests can be fixed. I was working up to the group level and that was the next step here. This idea would have worked fine at the individual and group level.

@TomDonoghue TomDonoghue reopened this Feb 23, 2021
@ryanhammonds ryanhammonds mentioned this pull request Mar 9, 2021
3 tasks
@TomDonoghue TomDonoghue added idea / discussion A potential idea to consider / discuss. >2.0 Idea for beyond specparam 2.0. labels Jun 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
idea / discussion A potential idea to consider / discuss. >2.0 Idea for beyond specparam 2.0.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants