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

Fix synapse filt to allow list as input #1123

Merged
merged 1 commit into from
Jul 15, 2016
Merged

Fix synapse filt to allow list as input #1123

merged 1 commit into from
Jul 15, 2016

Conversation

arvoelke
Copy link
Contributor

@arvoelke arvoelke commented Jul 15, 2016

Description:
Fixes a bug where Synapse.filt could not accept a list as input.

Motivation and context:
Lowpass(0.005).filt([1, 0, 0]) would raise the error AttributeError: 'list' object has no attribute 'dtype'. There is no issue for this bug.

How has this been tested?
Changed a test from u -> list(u).

Where should a reviewer start?
One line in synapses.py.

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

@mention-bot
Copy link

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

@arvoelke
Copy link
Contributor Author

arvoelke commented Jul 15, 2016

Let me know if it's worthwhile to add a test and then I can, but in general it seems silly to test lists on array_like interfaces unless there's something critical going on. Also didn't think this was worthy of bloating the changelog.

@tbekolay
Copy link
Member

I'd say rather than adding a test, modify an existing tests to use a list (just call tolist or something). Why don't you think this is changelog worthy?

@arvoelke
Copy link
Contributor Author

Done both.

@hunse
Copy link
Collaborator

hunse commented Jul 15, 2016

LGTM

@jgosmann
Copy link
Collaborator

LGTM 🍰

@tbekolay
Copy link
Member

LGTM too, I'll merge 👍

@tbekolay tbekolay added the bug label Jul 15, 2016
@tbekolay tbekolay merged commit 4d7eb42 into master Jul 15, 2016
@tbekolay tbekolay deleted the filt-dtype-fix branch July 15, 2016 15:56
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.

None yet

5 participants