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

Translate improvements #62

Merged
merged 5 commits into from
Aug 20, 2017
Merged

Translate improvements #62

merged 5 commits into from
Aug 20, 2017

Conversation

jgosmann
Copy link
Collaborator

@jgosmann jgosmann commented Jun 30, 2017

Motivation and context:
This PR makes a couple of improvements to the vocabulary translation mechanism.

To make it possible to use the solver argument in action rules, I imported the whole nengo name space into the action rule built-ins. But it occured to me that this cannot be the final solution because solvers might be defined outside of nengo and these solvers should be usable to. However, making all globals accessible is also problematic because names can refer to modules or semantic pointers and at parse time this can currently not be determined (technically it should be possible, but will require some major changes to the parsing). Maybe the best option is to have the user explicitely populate the builtin namespace, but this will increase boiler plate code.

Interactions with other PRs:
none

How has this been tested?
added tests

How long should this take to review?

  • Average (neither quick nor lengthy)

Where should a reviewer start?
Might make sense to do this one in order of the commits.

Types of changes:

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

Checklist:

  • I have read the CONTRIBUTING.rst document.
  • [n/a] 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:

  • Figure out a better and more general way to provide access to solvers in action rules.

@jgosmann jgosmann self-assigned this Jun 30, 2017
@jgosmann jgosmann added this to the 0.3 release milestone Jun 30, 2017
@jgosmann jgosmann mentioned this pull request Jul 5, 2017
3 tasks
This removes also the possibility to pass in a locals dict
instead of the stacklevel to retrieve the locals from. This
simplifies the code a bit and gives the argument a clearer name.
@jgosmann jgosmann force-pushed the translate-improvements branch from c1b3d0b to 087512b Compare August 20, 2017 17:11
This will make it easier to support a solver argument as proposed
in #57.
That can give better matrices to transform between vocabularies.

Addresses #57.
@jgosmann jgosmann merged commit b3e6d14 into master Aug 20, 2017
@jgosmann jgosmann deleted the translate-improvements branch August 20, 2017 18:32
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.

Add argument to ignore missing keys in translate
2 participants