fooof icon indicating copy to clipboard operation
fooof copied to clipboard

[WIP] - Name update for new version

Open TomDonoghue opened this issue 4 years ago • 1 comments

This PR is breaking update to overhaul the naming scheme of the module.

Principally, this includes changing the module name from fooof -> specparam, and all associated changes.

A note on reviewing

This PR necessarily has an extreme number changes, and it's not necessarily particularly easy or useful to try and read all the diffs. At this point, the main points that would be useful / interesting are to read through the name updates listed here and see if they make sense, and then perhaps install the module (pip install . works), and stress test the module, that things work as expected and naming and usage is user friendly.

Since this is a breaking update - now is a great time to dig in and change anything we want. So any ideas of changes to add, throw them out!

Name Changes

Objects: FOOOF -> PSD FOOOFGroup -> PSDGroup

Model attributes: fooofed_spectrum_ -> modeled_spectrum

Group: get_fooof -> get_model

Data objects: FOOOFResults -> FitResults FOOOFSettings -> ModelSettings FOOOFMetaData -> SpectrumMetaData

Functions combine_fooofs -> combine_model_objs compare_info -> compare_model_objs average_fg -> average_group fit_fooof_3d -> fit_models_3d

get_band_peak_fm -> get_band_peak get_band_peak_fg -> get_band_peak_group get_band_peak -> get_band_peak_arr get_band_peak_group -> get_band_peak_group_arr

compute_pointwise_error_fm -> compute_pointwise_error compute_pointwise_error_fg -> compute_pointwise_error_group compute_pointwise_error -> compute_pointwise_error_arr

save_fm -> save_model save_fg -> save_group

fetch_fooof_data -> fetch_example_data load_fooof_data -> load_example_data

gen_power_spectrum -> sim_power_spectrum gen_group_power_spectra -> sim_group_power_spectra

Function inputs: fooof_obj -> model_obj

Notes

An alternative option for the object names is SpecParam and SpecParamGroup (or similar), which I think Ryan suggested. I don't think there's a huge difference, with just slight pros & cons between the options. PSD is shorter (a pro, I think), though somewhat less clear its a model object. I like the notion of having the objects be a different name from the module name (it's easier to search through code, etc, which became salient doing this update). We could even consider PSDModel or something.

Where relevant, we are using US (for example 'modeled'), even though this makes me sad.

ToDos

  • URLs that point to things like the docsite still need to be updated

TomDonoghue avatar Apr 16 '21 22:04 TomDonoghue

Whew! Okay I looked through all the diffs, and this looks great. The only remaining question I have relates to your original PR comment about the name of PSD, which to me reads as too ambiguous and unclear that it's a model. So I would vote for changing PSD to PSDModel or similar, as you mentioned. Also, nit-picky, but it doesn't require that the input be a power spectral density: it just needs some spectral representation of the data. So if we're going for generality, SpectralModel might be even better.

voytek avatar May 10 '21 17:05 voytek

Names look good to me, with a minor preference for SpecParam and SpecParamGroup (I don't know if you've decided on this already)

rdgao avatar Aug 19 '23 23:08 rdgao