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

Moved gate, channel, convolution network to target module #906

Merged
merged 2 commits into from
Nov 27, 2015
Merged

Conversation

s72sue
Copy link
Contributor

@s72sue s72sue commented Nov 24, 2015

Fix for #894

@s72sue
Copy link
Contributor Author

s72sue commented Nov 24, 2015

Does anyone know what this error means?

@ikajic
Copy link
Contributor

ikajic commented Nov 24, 2015

If you click on "Details" in box, you'll see a little red cross in a build job that failed. Now, you can click on that job and see that there's this line:

nengo/spa/thalamus.py:153:30: E128 continuation line under-indented for visual indent

so the error simply means that the code-style is not conforming to PEP standards and flake package checks for that. If you just indent synapse=self.synapse_to_gate, transform=-1) in thalamus.py like it was before (so it is right below the bracket ( in the line before), my guess is that this should pass.

@Seanny123
Copy link
Contributor

After you fix the syntax error, this pull request looks good to me. Thanks for tackling the problem Su!

@s72sue
Copy link
Contributor Author

s72sue commented Nov 24, 2015

Thanks Ivana and Sean!
@Seanny123 , You might want to take another look since I made some changes to the cortical. Since the conv network was moved to the target, it didn't make sense to keep the bias node in cortical, so I have moved that to the target (as required by #905).

However, the cortical still shows up. Is that because nengo_gui creates a component for each spa module? If yes, should it be checking whether the module is empty?

@Seanny123
Copy link
Contributor

Yeah, that's a bug for Nengo GUI to deal with.

@Seanny123
Copy link
Contributor

@tbekolay are we still supposed to ping you for merges?

@tbekolay
Copy link
Member

Eric and Dan can merge as well; anyone on the maintainers team. But yeah, I
can merge this tomorrow morning!
On Nov 25, 2015 6:13 PM, "Sean Aubin" notifications@github.com wrote:

@tbekolay https://github.com/tbekolay are we still supposed to ping you
for merges?


Reply to this email directly or view it on GitHub
#906 (comment).

@tbekolay
Copy link
Member

This makes some API changes, but they're unlikely affect many people, so I'm going to add a short changelog entry. Aside from that, looks great!

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