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.

Sign up for GitHub

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

Jump to bottom

Fix Connection transform validation #1326

Merged
merged 1 commit into from
Jun 23, 2017
Merged

Fix Connection transform validation #1326

merged 1 commit into from
Jun 23, 2017

Conversation

drasmuss
Copy link
Member

@drasmuss drasmuss commented Jun 15, 2017 •
edited
Loading

It should raise an error if transform=None (since optional=False), but it wasn't because it was being cast to np.array(None).

How has this been tested?
Added a new test, existing tests pass.

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

tbekolay
tbekolay approved these changes Jun 15, 2017
View reviewed changes
Copy link
Member

@tbekolay tbekolay left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@tbekolay
Copy link
Member

FYI Appveyor failure will be fixed when rebased to current master (fixed in #1323).

@drasmuss @tbekolay
Fix Connection transform validation
e3e2a52 
It should raise an error if transform=None and optional=False.
@tbekolay tbekolay merged commit e3e2a52 into master Jun 23, 2017
@tbekolay tbekolay deleted the none_transform branch June 23, 2017 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tbekolay tbekolay tbekolay approved these changes

@jgosmann jgosmann jgosmann approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@drasmuss @tbekolay @jgosmann