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

Refactor conngen module to be a ConnBuilder #1830

Merged
merged 51 commits into from
Nov 23, 2020

Conversation

jougs
Copy link
Contributor

@jougs jougs commented Nov 7, 2020

This PR refactors the Connection Generator interface for NEST to be a regular ConnBuilder called conngen instead of being an extension module called conngen, which used to provide the CGConnect function. This addresses one of the action items of #1717.

@jougs jougs self-assigned this Nov 7, 2020
@jougs jougs added I: Behavior changes Introduces changes that produce different results for some users S: Normal Handle this with default priority T: Maintenance Work to keep up the quality of the code and documentation. labels Nov 7, 2020
@jougs jougs added this to the NEST 3.0 milestone Nov 7, 2020
@stinebuu stinebuu self-requested a review November 9, 2020 08:55
Copy link
Contributor

@sarakonradi sarakonradi left a comment

Choose a reason for hiding this comment

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

Thanks! I've added some documentation-related suggestions inline.

Comment on lines 208 to 209
library, NEST supports the Connection Generator Interface (`Djurfeldt
et al., 2014 <https://doi.org/10.3389/fninf.2014.00043>`_).
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we use an RST footnote reference instead, i.e., writing Connection Generator Interface [1]_ and adding a "References" section at the bottom?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea. Done in d5802c9.

examples/nest/conngen_interface.sli Outdated Show resolved Hide resolved
pynest/examples/csa_spatial_example.py Outdated Show resolved Hide resolved
@jougs
Copy link
Contributor Author

jougs commented Nov 13, 2020

@stinebuu, @sarakonradi: I've addressed all your points in my recent commits. Not sure if we really need the review by @hakonsbm. If you think we're good without, feel free to say so or remove the request of review.

@jougs jougs requested review from stinebuu and sarakonradi and removed request for hakonsbm November 13, 2020 13:33
Copy link
Contributor

@stinebuu stinebuu left a comment

Choose a reason for hiding this comment

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

Thank you!

Copy link
Contributor

@sarakonradi sarakonradi left a comment

Choose a reason for hiding this comment

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

LGTM!

@stinebuu stinebuu merged commit 8f5a5fc into nest:master Nov 23, 2020
@jougs jougs deleted the refactor_conngen_module branch August 31, 2023 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I: Behavior changes Introduces changes that produce different results for some users S: Normal Handle this with default priority T: Maintenance Work to keep up the quality of the code and documentation.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants