340 interface additional cf returns #343
Conversation
- we have to parse and serialize 0-dim arrays it seems Changes to be committed: modified: syncopy/nwanalysis/AV_compRoutines.py modified: syncopy/nwanalysis/connectivity_analysis.py modified: syncopy/nwanalysis/granger.py modified: syncopy/nwanalysis/wilson_sf.py modified: syncopy/shared/metadata.py modified: syncopy/tests/backend/test_conn.py
Changes to be committed: modified: syncopy/shared/metadata.py
Changes to be committed: modified: syncopy/nwanalysis/AV_compRoutines.py modified: syncopy/shared/metadata.py modified: syncopy/tests/test_connectivity.py
- no complicated trialdefinition needed a keeptrials=False is enforced in frontend Changes to be committed: modified: syncopy/nwanalysis/ST_compRoutines.py modified: syncopy/nwanalysis/connectivity_analysis.py
Changes to be committed: modified: syncopy/specest/compRoutines.py
- further beefed up `propagate_properties` to improve DRY - single-trial cross-spectra frequency hashes now also get checked Changes to be committed: modified: syncopy/nwanalysis/AV_compRoutines.py modified: syncopy/nwanalysis/ST_compRoutines.py modified: syncopy/nwanalysis/csd.py modified: syncopy/preproc/compRoutines.py modified: syncopy/shared/computational_routine.py modified: syncopy/shared/metadata.py modified: syncopy/specest/compRoutines.py modified: syncopy/tests/test_connectivity.py modified: syncopy/tests/test_specest.py
- renaming is hard.. Changes to be committed: modified: syncopy/preproc/compRoutines.py
Changes to be committed: modified: syncopy/nwanalysis/ST_compRoutines.py modified: syncopy/tests/backend/test_conn.py modified: syncopy/tests/backend/test_fooofspy.py
- I should play the lottery.. Changes to be committed: modified: syncopy/tests/backend/test_fooofspy.py
Codecov Report
@@ Coverage Diff @@
## dev #343 +/- ##
==========================================
- Coverage 69.09% 68.66% -0.43%
==========================================
Files 77 77
Lines 8850 8870 +20
Branches 1816 1823 +7
==========================================
- Hits 6115 6091 -24
- Misses 2319 2351 +32
- Partials 416 428 +12
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
| out.trialdefinition = trl | ||
| else: | ||
| out.trialdefinition = np.array([[0, 1, 0]]) | ||
| check_freq_hashes(metadata, out) |
There was a problem hiding this comment.
good idea to put this into a function
| @@ -1019,15 +1019,14 @@ def process_metadata(self, data, out): | |||
|
|
|||
| # --- metadata helper functions --- | |||
|
|
|||
| def propagate_metadata(in_data, out_data): | |||
| def propagate_properties(in_data, out_data, keeptrials=True): | |||
There was a problem hiding this comment.
very nice, this reduces quite a bit code duplication
There was a problem hiding this comment.
As discussed, it's still not perfect imho.. I could also think of putting those property mappings into the respective module compRoutines.py, however there is already quite some crossover between freqanalysis and connectivityanalysis for example.
syncopy/tests/backend/test_conn.py
Outdated
| CSDreg, fac, iniCN = regularize_csd(CSD, cond_max=cmax, nSteps=25) | ||
| # get initial CSD condition number, which is way too large! | ||
| print(iniCN) | ||
| assert iniCN > cmax |
There was a problem hiding this comment.
optional: could be nice to use 2nd arg of assert statement to print better error here, like
assert iniCN > cmax, f"some message about var {var} here
There was a problem hiding this comment.
yes, I've seen this only in your code now.. did not know this is possible, but good idea 👍
There was a problem hiding this comment.
puh, there was definitely something off in any case.. I corrected it, still not sure how often ill conditioned CSDs appear in the wild 😅
But at least now we know at least aposteriori what the factorization/regularization did thanks to the additional cF returns :)
|
Looks good, only 2 things:
|
|
About your 2nd point regarding the docstring of the frontend, I now think we should programmatically fix/define the key's which are propagated from the CR/cF to |
- corrected regularization test - increased maximal Granger regularization parameter to 0.1 - updated Changelog - added .info Granger returns to frontend doc Changes to be committed: modified: CHANGELOG.md modified: syncopy/nwanalysis/AV_compRoutines.py modified: syncopy/nwanalysis/connectivity_analysis.py modified: syncopy/tests/backend/test_conn.py
|
I also added the changelog entries for the additional fooof returns, which are also new right :) ?! Feel free to edit/change! |
Make use of additional cF returns
.infodocumenting the factorizationprocess_metadatato reduce copied code and have one place where the properties get attached from input to output (propagate_properties)closes #148
closes #340
Author Guidelines
Reviewer Checklist