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

Run modes #292

Closed
mwprestonjr opened this issue Jul 21, 2023 · 4 comments
Closed

Run modes #292

mwprestonjr opened this issue Jul 21, 2023 · 4 comments
Labels
1.1 Targetted for a fooof 1.1 release.

Comments

@mwprestonjr
Copy link
Contributor

FOOOFGroup.get_group() does not set the run modes (_debug, _check_freqs, _check_data) to match that of the input object, which can cause errors; while FOOOFGroup.get_fooof() sets the data mode only. Including "run_modes" as a key within the attributes dictionary returned by core/info/get_description() may facilitate tracking of these attributes.
Also, _check_freqs is not included in the docstring of fit.py.

@TomDonoghue TomDonoghue added the 1.1 Targetted for a fooof 1.1 release. label Jul 21, 2023
@TomDonoghue
Copy link
Member

Ahhh, ooops, yes - good catch! I'll fix this - should be easy to propogate!

@SM-Figueroa
Copy link
Contributor

SM-Figueroa commented Jul 22, 2023

Hi @TomDonoghue. I'm an undergraduate researcher working with MJ. I've already started on a fix for this issue and would be happy to open a pull request in the near future. Please let me know if that's acceptable. Thanks.

Follow up: should there be a setter for the _check_freqs attribute?

@TomDonoghue
Copy link
Member

@SM-Figueroa - thanks for the interest! Apologies, I only saw this message after I'd already gone ahead and done the updates in #293, so there is already some stuff related to this in progress.

I had a look at your branch and I like what you are doing with set_run_modes and so on, I think it's a nice and clean implementation. Can you open a PR with what you have, and I can help merge what you've added that is beyond #293?

@TomDonoghue
Copy link
Member

This is now addressed in #293 - so moving discussion over there and closing this here now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.1 Targetted for a fooof 1.1 release.
Projects
None yet
Development

No branches or pull requests

3 participants