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 standardized network inputs and outputs #1296

Merged
merged 2 commits into from
Apr 18, 2017
Merged

Conversation

jgosmann
Copy link
Collaborator

@jgosmann jgosmann commented Apr 17, 2017

Motivation and context:
This makes the product and circular convolution network adhere to the newly defined standard for network inputs (#887) which should be named input_*. The old inputs are still available to keep backwards compatibility. They will not produce a deprecation warning as this is not possible in a backwards compatible way. Furthermore, this PR deprecates the net argument. As discussed before this argument was rarely used and in almost all uses cases the new **kwargs passed to nengo.Network provide the same functionality. This is required to allow for a deprecation on the old inputs in the future.

For consistency I made the net argument also obsolete on other network creation functions. In some other networks I also introduced in output node to provide a common interface.

Interactions with other PRs:

new-spa/nengo_spa uses the new input names.
This is a non-breaking version of #1179 for the inclusion in one of the next minor releases. #1179 is for the inclusion in the next major release.

How has this been tested?
Unit tests still pass.

How long should this take to review?

  • Average (neither quick nor lengthy)

Types of changes:

  • New feature (non-breaking change which adds functionality)

Checklist:

  • I have read the CONTRIBUTING.rst document.
  • I have updated the documentation accordingly.
  • I have included a changelog entry.
  • [n/a] I have added tests to cover my changes.
  • All new and existing tests passed.

Copy link
Contributor

@Seanny123 Seanny123 left a comment

Choose a reason for hiding this comment

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

Yay! Standardization! LGTM.

@tcstewar tcstewar self-requested a review April 18, 2017 01:12
Copy link
Contributor

@tcstewar tcstewar left a comment

Choose a reason for hiding this comment

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

This also looks good to me! Thanks!

This makes the product and circular convolution network adhere to the
newly defined standard for network inputs which should be named
'input_*'.

This does not touch the names of inputs to the SPA networks as most of
them will be refactored in nengo_spa anyways.

The old names are still accessible. For technical reasons, they do not
raise a DeprecationWarning, though that would have been preferable.

Addresses #887.
Copy link
Member

@tbekolay tbekolay left a comment

Choose a reason for hiding this comment

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

Amended in changelog entries here too. Will merge when CI finishes.

@tbekolay tbekolay merged commit 0eb77e6 into master Apr 18, 2017
@tbekolay tbekolay deleted the net-inputs-non-breaking branch April 18, 2017 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants