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

Declare entry point for nengo reference simulator. #1127

Merged
merged 1 commit into from
Jul 27, 2016

Conversation

jgosmann
Copy link
Collaborator

@jgosmann jgosmann commented Jul 22, 2016

Description:
This adds an entry point for the reference simulator so that other Python packages (like nengo_gui) can discover the backend in a general way. Such an entry point should become best practice for all backends.

Motivation and context:
See description.

How has this been tested?

  1. Checkout the branch and do a python setup.py develop.
  2. Run the following Python code:
from pkg_resources import iter_entry_points
print([ep.load() for ep in iter_entry_points(group='nengo.backends')])

It will print "[<class 'nengo.simulator.Simulator'>]".

Where should a reviewer start?
Click "Files changed".

How long should this take to review?

  • Quick (less than 40 lines changed or changes are straightforward)

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.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Still to do:

  • Add some documentation, but we need some general backend developer documentation first. I'll create an issue for that.

@mention-bot
Copy link

@jgosmann, thanks for your PR! By analyzing the annotation information on this pull request, we identified @tbekolay to be a potential reviewer

@tbekolay
Copy link
Member

It will print "[]".

I get [<class 'nengo.simulator.Simulator'>] which I'm pretty sure is actually what we want ;)

Aside from that, this looks good to me! I pushed a commit adding a test.

@jgosmann
Copy link
Collaborator Author

That's the markdown messing with me ... click the edit icon and you will see I put in in there. I'm innocent. I promise. 😇

@jgosmann
Copy link
Collaborator Author

It would also be possible to load a specific entry point without iterating over all. Might that be more appropriate for the test?

@tbekolay
Copy link
Member

The advantage of loading them all (which shouldn't take long) is that we don't have to write a different test for each backend; also, failing this test will let backends know that they should also declare this entry point.

@jgosmann
Copy link
Collaborator Author

Good point!

@Seanny123
Copy link
Contributor

Given that the only test that is not passing was fixed by #1134, this branch looks ready to merge.

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