Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Updates to match changes in Nengo #44

Merged
merged 1 commit into from
Nov 26, 2019
Merged

Updates to match changes in Nengo #44

merged 1 commit into from
Nov 26, 2019

Conversation

bmorcos
Copy link
Contributor

@bmorcos bmorcos commented Nov 6, 2019 •
edited
Loading

Motivation and context:

Nengo has made some breaking changes, this brings NengoFPGA back into compatibility.

How has this been tested?

  • Tested with basic exampe and notebooks with full stack.
  • Have not tested GUI yet (pending GUI patch)

How long should this take to review?

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

Types of changes:

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • I have read the CONTRIBUTING.rst document.
  • N/A I have updated the documentation accordingly.
  • I have included a changelog entry.
  • I have tested this with all supported devices.
  • N/A I have added tests to cover my changes.
  • N/A I have run the test suite locally and all tests passed.

Still to do:

  • Test GUI examples

@bmorcos
Copy link
Contributor Author

bmorcos commented Nov 6, 2019

TravisCI can't pip install Nengo>=3.0.0... maybe we just let this sit here until the 3.0 release?

@bmorcos
Copy link
Contributor Author

bmorcos commented Nov 6, 2019

Similarly, nengo-gui needed to be patched for compat. and nengo_extras...

@bmorcos
Copy link
Contributor Author

bmorcos commented Nov 6, 2019 •
edited
Loading

Having some trouble getting the (compatible) GUI to log info...

I can grab the GUI logger and set the log level for everything:

logger = logging.getLogger()
logger.setLevel("INFO")

But it seems like the GUI is unable to handle the separate NengoFPGA logger.

xchoo
xchoo reviewed Nov 7, 2019
View reviewed changes
Copy link
Member

@xchoo xchoo left a comment

Choose a reason for hiding this comment

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

It looks like you need to run the notebook cleaner script on the notebooks?

@bmorcos bmorcos mentioned this pull request Nov 7, 2019
Merged
3 tasks
bmorcos
bmorcos commented Nov 7, 2019
View reviewed changes
CHANGES.rst
@@ -26,6 +26,10 @@ Release History
- Setup Nengo Bones and remote CI.
(`#41 <https://github.com/nengo/nengo-fpga/pull/41>`__)

**Changed**
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we put this under the **Fixed ** header instead maybe?

Copy link
Member

Choose a reason for hiding this comment

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

Changed is okay. But the changelog should specify the changes were made for Nengo 3.0.0.

xchoo
xchoo reviewed Nov 8, 2019
View reviewed changes
Copy link
Member

@xchoo xchoo left a comment

Choose a reason for hiding this comment

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

I would wait for Travis-CI to successfully build with Nengo 3.0.0 before merging this.

CHANGES.rst
@@ -26,6 +26,10 @@ Release History
- Setup Nengo Bones and remote CI.
(`#41 <https://github.com/nengo/nengo-fpga/pull/41>`__)

**Changed**
Copy link
Member

Choose a reason for hiding this comment

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

Changed is okay. But the changelog should specify the changes were made for Nengo 3.0.0.

@xchoo
Copy link
Member

xchoo commented Nov 25, 2019 •
edited
Loading

Why is this PR complaining about conflicting files? @bmorcos have you seen this before? Shouldn't conflicts be addressed during the merge process?

@bmorcos
Copy link
Contributor Author

bmorcos commented Nov 25, 2019

Yea, I think this is just saying it can't automatically merge from the GUI since there are conflicts (this is a commit behind master. So yea, should all be fixed in the manual merge I imagine.

@bmorcos
Copy link
Contributor Author

bmorcos commented Nov 25, 2019

I patched the URLs for now, but I made a note to start using proper intersphinx references.

xchoo
xchoo requested changes Nov 26, 2019
View reviewed changes
Copy link
Member

@xchoo xchoo left a comment

Choose a reason for hiding this comment

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

Since we are patching it to work with the new Nengo docs release, there are also links to parts of the nengo docs in our example notebooks (01 and 02) that need changing.

@bmorcos
Copy link
Contributor Author

bmorcos commented Nov 26, 2019

I patched those URLs also, did I miss something?

@bmorcos
Updates to match changes in Nengo 3.0.0
97fe1ba 
- Fixes #42, #43
- No more nengo.compat
  - Use configparser directly
  - Use nengo.utils.numpy.is_array_like
- Add new state arg to make_step
- Start using native logger instead of nengo logger in examples
- Update requirements to nengo>=3.0.0
- Update nengo urls to us '-' instead of '_'
@bmorcos bmorcos merged commit 97fe1ba into master Nov 26, 2019
@bmorcos bmorcos deleted the nengo-patches branch November 26, 2019 15:29
@bmorcos bmorcos mentioned this pull request Nov 27, 2019
Closed
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@xchoo xchoo xchoo approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@bmorcos @xchoo