-
Notifications
You must be signed in to change notification settings - Fork 17
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
Data class refactor #161
Merged
Merged
Data class refactor #161
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
…ing method (will add back).
cgohil8
force-pushed
the
data_class
branch
2 times, most recently
from
June 29, 2023 21:15
89d5f99
to
6910f5e
Compare
This commit reproduces the old code when doing the following: - Calling .prepare(n_embeddings=15, n_pca_components). This means the new tde_pca and standardize methods are working. - Calling .filter(low_freq=1, high_freq=10). - Calling .filter(low_freq=1, low_freq=10);.amp_env(); .sliding_window(n_window=5);.standardize() reproduces .prepare(amplitude_envelope=True, low_freq=1, high_freq=10, n_window=5). This means all the preparation methods reproduce the old code.
cgohil8
force-pushed
the
data_class
branch
2 times, most recently
from
June 29, 2023 22:16
860a87b
to
d4ad2fb
Compare
cgohil8
force-pushed
the
data_class
branch
2 times, most recently
from
June 30, 2023 10:44
dd0a6ed
to
9fa5ff6
Compare
In particular, I checked: methods = { "tde_pca": {"n_embeddings": 15, "n_pca_components": 80}, "standardize": {}, } data.prepare(methods) produces exactly the same data as (in the old code): data.prepare(n_embeddings=15, n_pca_components=80) and methods = { "filter": {"low_freq": 1, "high_freq": 30}, "amplitude_envelope": {}, "sliding_window": {"n_window": 5}, "standardize": {}, } data.prepare(methods) produces exactly the same data as (in the old code): data.prepare(amplitude_envelope=True, low_freq=1, high_freq=30, n_window=5)
To implement true chaining ( |
evanr70
reviewed
Jun 30, 2023
…the same length as get_alpha(..., remove_edge_effects=False).
evanr70
reviewed
Jun 30, 2023
evanr70
reviewed
Jun 30, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll make a few operational changes to pipeline and rw, but excellent work overall
evanr70
approved these changes
Jun 30, 2023
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.
Massive refactor of the Data class
Changes:
Data.prepare
method and theload_data
wrapper in the config API..prepare()
.Preparation methods
We can now chain methods, e.g. for TDE-PCA data preparation we use:
To prepare amplitude envelope data we use:
Tests confirm all new data preparation methods produce the same data as old code.
The
Data.prepare
method now takes adict
which contains methods to call in series. E.g.(The order of the
dict
should be preserved from creation - added in Python 3.6+)Saving and loading
Internal variables names were changed significantly. In particular, the
Data.save
method was changed significantly. However, the changes to saving and loading are backwards compatible, data saved with old code will still be readable.Potential improvements
Rather than defining
_apply
functions in each method, we could see if we can usefunctools.partial
or define a decorator for the functions indata.processing
.