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.

Sign up for GitHub

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

Jump to bottom

Add functions yielding source neurons given target neurons #2394

Merged
merged 34 commits into from
Sep 5, 2022
Merged

Add functions yielding source neurons given target neurons #2394

merged 34 commits into from
Sep 5, 2022

Conversation

ackurth
Copy link
Contributor

@ackurth ackurth commented May 11, 2022

In my work with spatial networks,I find it oftentimes useful to have an easy interface for accessing, plotting etc. the source neurons of a given target neuron.
Currently, while there are function to get information of target neurons given source neurons nest.GetTargetNodes, nest.GetTargetPositions as well as a plotting routine nest.PlotTargets, functionality regarding the other way (i.e. about source neurons given a target neuron) is lacking.
This PR addresses this and introduces nest.GetSourceNodes, nest.GetSourcePositions, nest.PlotTargets for the NEST Python API.
Or is there a reason I don't see yet for not having these functions as well?

@terhorstd terhorstd added T: Enhancement New functionality, model or documentation S: Normal Handle this with default priority I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) labels Jun 27, 2022
@terhorstd terhorstd added this to In progress in PyNEST via automation Jun 27, 2022
@terhorstd terhorstd requested a review from hakonsbm June 27, 2022 17:20
hakonsbm
hakonsbm suggested changes Jul 6, 2022
View reviewed changes
Copy link
Contributor

@hakonsbm hakonsbm left a comment

Choose a reason for hiding this comment

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

Thanks for these useful functions. I have just a few minor suggestions.

doc/htmldoc/networks/spatially_structured_networks.rst Outdated Show resolved Hide resolved
pynest/examples/spatial/nodes_source_target.py Outdated Show resolved Hide resolved
pynest/nest/lib/hl_api_spatial.py Outdated Show resolved Hide resolved
pynest/nest/lib/hl_api_spatial.py Outdated Show resolved Hide resolved
pynest/nest/lib/hl_api_spatial.py Outdated Show resolved Hide resolved
pynest/nest/lib/hl_api_spatial.py Outdated Show resolved Hide resolved
testsuite/pytests/test_spatial/test_basics.py Outdated Show resolved Hide resolved
testsuite/pytests/test_spatial/test_basics.py Show resolved Hide resolved
testsuite/pytests/test_spatial/test_plotting.py Outdated Show resolved Hide resolved
testsuite/pytests/test_spatial/test_basics.py Outdated Show resolved Hide resolved
PyNEST automation moved this from In progress to Review Jul 6, 2022
@terhorstd
Copy link
Contributor

This seems not critical and might need a bit more time for a reaction, since @ackurth is busy at the moment.
All in good time.

@heplesser heplesser self-requested a review August 18, 2022 08:06
heplesser
heplesser approved these changes Aug 18, 2022
View reviewed changes
Copy link
Contributor

@heplesser heplesser left a comment

Choose a reason for hiding this comment

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

Thanks for this PR. It looks largely fine, but I have added some suggestions.

doc/htmldoc/networks/spatially_structured_networks.rst Show resolved Hide resolved
doc/htmldoc/networks/spatially_structured_networks.rst Outdated Show resolved Hide resolved
pynest/examples/spatial/nodes_source_target.py Outdated Show resolved Hide resolved
pynest/examples/spatial/nodes_source_target.py Outdated Show resolved Hide resolved
pynest/nest/lib/hl_api_spatial.py Outdated Show resolved Hide resolved
pynest/nest/lib/hl_api_spatial.py Show resolved Hide resolved
pynest/nest/lib/hl_api_spatial.py Outdated Show resolved Hide resolved
pynest/nest/lib/hl_api_spatial.py Show resolved Hide resolved
ackurth and others added 7 commits August 28, 2022 18:06
@ackurth @hakonsbm
Update doc/htmldoc/networks/spatially_structured_networks.rst
32fffdd 
Co-authored-by: Håkon Bakke Mørk <hakon.mork@nmbu.no>
@ackurth @hakonsbm
Update pynest/examples/spatial/nodes_source_target.py
325763b 
Co-authored-by: Håkon Bakke Mørk <hakon.mork@nmbu.no>
@ackurth @hakonsbm
Update pynest/nest/lib/hl_api_spatial.py
201bba3 
Co-authored-by: Håkon Bakke Mørk <hakon.mork@nmbu.no>
@ackurth @hakonsbm
Update pynest/nest/lib/hl_api_spatial.py
5037c94 
Co-authored-by: Håkon Bakke Mørk <hakon.mork@nmbu.no>
@ackurth
Format lines in docstring to fit in one line
7617304
@ackurth
Merge remote-tracking branch 'origin/add_functions_on_sources' into a…
124617e 
…dd_functions_on_sources
@ackurth @hakonsbm
Update pynest/nest/lib/hl_api_spatial.py
17af74d 
Co-authored-by: Håkon Bakke Mørk <hakon.mork@nmbu.no>
ackurth and others added 7 commits August 28, 2022 18:36
@ackurth @hakonsbm
Update testsuite/pytests/test_spatial/test_basics.py
0521a5f 
Co-authored-by: Håkon Bakke Mørk <hakon.mork@nmbu.no>
@ackurth
Put Source before Target method in htmldoc
599eafd
@ackurth @heplesser
Update pynest/examples/spatial/nodes_source_target.py
06b3515 
Co-authored-by: Hans Ekkehard Plesser <hans.ekkehard.plesser@nmbu.no>
@ackurth @heplesser
Update pynest/examples/spatial/nodes_source_target.py
9873515 
Co-authored-by: Hans Ekkehard Plesser <hans.ekkehard.plesser@nmbu.no>
@ackurth @heplesser
Update pynest/nest/lib/hl_api_spatial.py
662cc6e 
Co-authored-by: Hans Ekkehard Plesser <hans.ekkehard.plesser@nmbu.no>
@ackurth @heplesser
Update pynest/nest/lib/hl_api_spatial.py
78a13f4 
Co-authored-by: Hans Ekkehard Plesser <hans.ekkehard.plesser@nmbu.no>
@ackurth
Update docstring of Get{Source,Target}* methods
c034c34
@ackurth
Copy link
Contributor Author

ackurth commented Aug 28, 2022

Thanks for the very thorough review and the nice comments.
I hope I addressed all the issues you had.

@ackurth ackurth requested review from hakonsbm and heplesser and removed request for hakonsbm and heplesser August 28, 2022 17:54
hakonsbm
hakonsbm suggested changes Aug 29, 2022
View reviewed changes
Copy link
Contributor

@hakonsbm hakonsbm left a comment

Choose a reason for hiding this comment

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

Thanks! See my reply of my previous review.

@hakonsbm hakonsbm mentioned this pull request Aug 29, 2022
Open
@ackurth
Copy link
Contributor Author

ackurth commented Aug 29, 2022

For some reason, after bringing the branch up to date with master, the static code check does not pass anymore.
If I understand the output correctly, the errors come from files I did not even tough?
I am confused.

@heplesser
Copy link
Contributor

For some reason, after bringing the branch up to date with master, the static code check does not pass anymore.
If I understand the output correctly, the errors come from files I did not even tough?
I am confused.

@ackurth, welcome to the mysteries of your formatting checker ... The test of unchanged files may be induced by the merge. The only fix is to correct the formatting ;).

@heplesser heplesser merged commit ffcbe05 into nest:master Sep 5, 2022
PyNEST automation moved this from Review to Done Sep 5, 2022
@jessica-mitchell jessica-mitchell mentioned this pull request Sep 7, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hakonsbm hakonsbm hakonsbm approved these changes

@heplesser heplesser heplesser approved these changes

Assignees
No one assigned
Labels
I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) S: Normal Handle this with default priority T: Enhancement New functionality, model or documentation
Projects
PyNEST
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@ackurth @terhorstd @heplesser @hakonsbm