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

Do not set default labels #235

Merged
merged 3 commits into from
Nov 18, 2019
Merged

Do not set default labels #235

merged 3 commits into from
Nov 18, 2019

Conversation

jgosmann
Copy link
Collaborator

Motivation and context:
Given the nengo/nengo-gui#1002 has been merged, not setting any default labels will give more informative labels in the Nengo GUI. This also converts all networks to classes to be more consistent.

Interactions with other PRs:
none

How has this been tested?
existing tests

How long should this take to review?

  • Average (neither quick nor lengthy)

Most lines did just change in indentation (due to init being nested in the class) and have net renamed to self.

Types of changes:

  • Breaking change (fix or feature that causes existing functionality to change)

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.
  • I have run the test suite locally and all tests passed.

Addresses #234.

Made possible with the merge of nengo/nengo-gui#1002.
@jgosmann
Copy link
Collaborator Author

It would be great to get a review on this. @tbekolay @tcstewar @Seanny123

@Seanny123
Copy link
Collaborator

@jgosmann to make sure I understand this PR, to review, I should install the master branch of Nengo GUI and this branch of Nengo SPA. Then, I should see the better labels discussed in this PR and in the linked Nengo GUI PR?

@jgosmann
Copy link
Collaborator Author

Exactly. I made some example screenshots:

Without this PR, if no explicit label is set, all State modules (for example) are just labeled "State":
image

With this PR, if no explicit label is set, the modules are labeled based on the assigned variable:
image
If the module is not assigned to a global variable, the label defaults to the class name (e.g. State).

Independent of this PR, explicitly setting the label will display that label:
image

There is also somewhat graceful degradation with older Nengo GUI versions than the current master. If no explicit label is set, but the module is not assigned to a global variable, it will be labeled "networks[i]" (where i is some integer).

Copy link
Collaborator

@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.

Works as expected on Windows 10.

@jgosmann jgosmann merged commit 983d067 into master Nov 18, 2019
@jgosmann jgosmann deleted the labels branch November 18, 2019 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants