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

Add csd connectivity #416

Merged
merged 14 commits into from
Jan 19, 2023
Merged

Add csd connectivity #416

merged 14 commits into from
Jan 19, 2023

Conversation

kajal5888
Copy link
Contributor

@kajal5888 kajal5888 commented Jan 11, 2023

Author Guidelines

  • Is the change set < 600 lines?
  • Was the code checked for memory leaks/performance bottlenecks?
  • Is the code running locally and on the ESI cluster?
  • Is the code running on all supported platforms?

Reviewer Checklist

  • Are testing routines present?
  • Do parallel loops have a set length and correct termination conditions?
  • Do objects in the global package namespace perform proper parsing of their input?
  • Do code-blocks provide novel functionality, i.e., no re-factoring using builtin/external packages possible?
  • Code layout
    • Is the code PEP8 compliant?
    • Does the code adhere to agreed-upon naming conventions?
    • Are keywords clearly named and easy to understand?
    • No commented-out code?
  • Are all docstrings complete and accurate?
  • Is the CHANGELOG.md up to date?

@kajal5888 kajal5888 requested a review from tensionhead January 11, 2023 11:04
Copy link
Contributor

@tensionhead tensionhead left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work @kajal5888 ! See my detailed comments below, I think most critical would be to rename the new_cfg entry for this frontend, as this right now breaks replayability.

You would also see this in a proper test however, would be great you could add a class TestCSD in /tests/test_connectivity.py. The easiest test I could think of, is to take the same synth data as in TestCoherence , and plug the new output generated with method='csd' into an existing csd normalization routine to recover the coherence from that csd.

syncopy/nwanalysis/connectivity_analysis.py Show resolved Hide resolved
syncopy/nwanalysis/connectivity_analysis.py Outdated Show resolved Hide resolved
syncopy/nwanalysis/connectivity_analysis.py Outdated Show resolved Hide resolved
syncopy/nwanalysis/connectivity_analysis.py Outdated Show resolved Hide resolved
syncopy/nwanalysis/connectivity_analysis.py Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Jan 11, 2023

Codecov Report

Base: 68.70% // Head: 68.04% // Decreases project coverage by -0.67% ⚠️

Coverage data is based on head (50e87f1) compared to base (35e635d).
Patch coverage: 82.14% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #416      +/-   ##
==========================================
- Coverage   68.70%   68.04%   -0.67%     
==========================================
  Files          79       80       +1     
  Lines        9610     9632      +22     
  Branches     1995     2010      +15     
==========================================
- Hits         6603     6554      -49     
- Misses       2502     2559      +57     
- Partials      505      519      +14     
Impacted Files Coverage Δ
syncopy/nwanalysis/connectivity_analysis.py 72.00% <82.14%> (+0.96%) ⬆️
syncopy/datatype/methods/show.py 61.66% <0.00%> (-13.34%) ⬇️
syncopy/datatype/discrete_data.py 66.55% <0.00%> (-11.76%) ⬇️
syncopy/datatype/methods/selectdata.py 76.04% <0.00%> (-2.69%) ⬇️
syncopy/shared/parsers.py 73.33% <0.00%> (-1.78%) ⬇️
syncopy/nwanalysis/wilson_sf.py 92.75% <0.00%> (-1.45%) ⬇️
syncopy/datatype/base_data.py 76.53% <0.00%> (-1.40%) ⬇️
syncopy/shared/kwarg_decorators.py 85.20% <0.00%> (-0.80%) ⬇️
syncopy/statistics/spike_psth.py 95.34% <0.00%> (-0.06%) ⬇️
syncopy/datatype/__init__.py 100.00% <0.00%> (ø)
... and 5 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

syncopy/tests/test_connectivity.py Show resolved Hide resolved
syncopy/tests/test_connectivity.py Outdated Show resolved Hide resolved
Copy link
Contributor

@tensionhead tensionhead left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thx @kajal5888 !

@tensionhead tensionhead linked an issue Jan 19, 2023 that may be closed by this pull request
@tensionhead tensionhead merged commit 99e723f into dev Jan 19, 2023
@tensionhead tensionhead deleted the Add_CSD_Connectivity branch March 31, 2023 10:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add method csd to connectivityanalysis
2 participants