-
Notifications
You must be signed in to change notification settings - Fork 100
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
[MNT] - Name update for new version #205
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
TomDonoghue
changed the title
[WIP] - Name update for new version
[MNT - Name update for new version
Aug 7, 2023
TomDonoghue
changed the title
[MNT - Name update for new version
[MNT] - Name update for new version
Aug 7, 2023
Names look good to me, with a minor preference for |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 -> SpectralModel
FOOOFGroup -> SpectralGroupModel
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 considerPSDModel
or something.Where relevant, we are using US (for example 'modeled'), even though this makes me sad.
ToDos