Skip to content

Cleanup TransferFunction Input Sanitization #135

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

Merged
murrayrm merged 14 commits into from
Jan 2, 2018
Merged

Cleanup TransferFunction Input Sanitization #135

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
37b06f4
Added notebook for doing development work on IO-Cleanup.
Feb 17, 2017
feb68f5
Added .eggs directory to gitignore.
Feb 17, 2017
5e2d95d
Broke out the numerator/denominator cleanup function into a separate …
Feb 18, 2017
cd7f596
Merge remote-tracking branch 'upstream/master'
Feb 18, 2017
509ff8e
Removed test notebook
Feb 18, 2017
d44489f
Renamed cleanPart to _cleanPart since it is an internal function, as …
Feb 18, 2017
b1437f0
unit test from kangwonlee commit facfe9c49d79227dd3bdcd22af9d0aa917ac…
murrayrm Dec 29, 2017
10cdb7a
TST: renamed unit test from kangwonlee
murrayrm Dec 29, 2017
af8837e
Merge branch 'InputSanitization' of https://github.com/jed-frey/pytho…
murrayrm Dec 29, 2017
b697a08
Merge branch 'jed-frey-InputSanitization' into sstf_input_sanitization
murrayrm Dec 29, 2017
68b5dd1
address additional comments from roryyorke in PR #135
murrayrm Dec 29, 2017
11a6264
Merge pull request #1 from murrayrm/sstf_input_sanitization
Dec 31, 2017
bbb902f
Merge branch 'master' into InputSanitization
murrayrm Jan 2, 2018
8c997ff
Merge branch 'master' into InputSanitization
murrayrm Jan 2, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,4 @@ examples/.ipynb_checkpoints/
.project
Untitled*.ipynb
*.idea/
.eggs
261 changes: 261 additions & 0 deletions control/tests/xferfcn_input_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,261 @@
#!/usr/bin/env python
#
# xferfcn_test.py - test TransferFunction class
# RMM, 30 Mar 2011 (based on TestXferFcn from v0.4a)

@roryyorke roryyorke Mar 17, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Probably worthwhile correcting this header comment


import unittest
import numpy as np

from numpy import int, int8, int16, int32, int64
from numpy import float, float16, float32, float64, float128
from numpy import all, ndarray, array

from control.xferfcn import _cleanPart

class TestXferFcnInput(unittest.TestCase):
"""These are tests for functionality of cleaning and validating
XferFucnInput."""

# Tests for raising exceptions.
def testBadInputType(self):
"""Give the part cleaner invalid input type."""

self.assertRaises(TypeError, _cleanPart, [[0., 1.], [2., 3.]])

def testBadInputType2(self):
"""Give the part cleaner another invalid input type."""
self.assertRaises(TypeError, _cleanPart, [1,"a"])

def testScalar(self):
"""Test single scalar value."""
num = 1
num_ = _cleanPart(num)

assert isinstance(num_, list)
assert np.all([isinstance(part, list) for part in num_])
np.testing.assert_array_equal(num_[0][0], array([1.0], dtype=float))

def testListScalar(self):
"""Test single scalar value in list."""
num = [1]
num_ = _cleanPart(num)

assert isinstance(num_, list)
assert np.all([isinstance(part, list) for part in num_])
np.testing.assert_array_equal(num_[0][0], array([1.0], dtype=float))

def testTupleScalar(self):
"""Test single scalar value in tuple."""
num = (1)
num_ = _cleanPart(num)

assert isinstance(num_, list)
assert np.all([isinstance(part, list) for part in num_])
np.testing.assert_array_equal(num_[0][0], array([1.0], dtype=float))

def testList(self):
"""Test multiple values in a list."""
num = [1, 2]
num_ = _cleanPart(num)

assert isinstance(num_, list)
assert np.all([isinstance(part, list) for part in num_])
np.testing.assert_array_equal(num_[0][0], array([1.0, 2.0], dtype=float))

def testTuple(self):
"""Test multiple values in tuple."""
num = (1, 2)
num_ = _cleanPart(num)

assert isinstance(num_, list)
assert np.all([isinstance(part, list) for part in num_])
np.testing.assert_array_equal(num_[0][0], array([1.0, 2.0], dtype=float))

def testAllScalarTypes(self):
"""Test single scalar value for all valid data types."""
for dtype in [int, int8, int16, int32, int64, float, float16, float32, float64, float128]:
num = dtype(1)
num_ = _cleanPart(num)

assert isinstance(num_, list)
assert np.all([isinstance(part, list) for part in num_])
np.testing.assert_array_equal(num_[0][0], array([1.0], dtype=float))

def testNpArray(self):
"""Test multiple values in numpy array."""
num = np.array([1, 2])
num_ = _cleanPart(num)

assert isinstance(num_, list)
assert np.all([isinstance(part, list) for part in num_])
np.testing.assert_array_equal(num_[0][0], array([1.0, 2.0], dtype=float))

def testAllNumpyArrayTypes(self):
"""Test scalar value in numpy array of ndim=0 for all data types."""
for dtype in [int, int8, int16, int32, int64, float, float16, float32, float64, float128]:
num = np.array(1, dtype=dtype)
num_ = _cleanPart(num)

assert isinstance(num_, list)
assert np.all([isinstance(part, list) for part in num_])
np.testing.assert_array_equal(num_[0][0], array([1.0], dtype=float))

def testAllNumpyArrayTypes2(self):
"""Test numpy array for all types."""
for dtype in [int, int8, int16, int32, int64, float, float16, float32, float64, float128]:
num = np.array([1, 2], dtype=dtype)
num_ = _cleanPart(num)

assert isinstance(num_, list)
assert np.all([isinstance(part, list) for part in num_])
np.testing.assert_array_equal(num_[0][0], array([1.0, 2.0], dtype=float))

def testListAllTypes(self):
"""Test list of a single value for all data types."""
for dtype in [int, int8, int16, int32, int64, float, float16, float32, float64, float128]:
num = [dtype(1)]
num_ = _cleanPart(num)
assert isinstance(num_, list)
assert np.all([isinstance(part, list) for part in num_])
np.testing.assert_array_equal(num_[0][0], array([1.0], dtype=float))

def testListAllTypes2(self):
"""List of list of numbers of all data types."""
for dtype in [int, int8, int16, int32, int64, float, float16, float32, float64, float128]:
num = [dtype(1), dtype(2)]
num_ = _cleanPart(num)
assert isinstance(num_, list)
assert np.all([isinstance(part, list) for part in num_])
np.testing.assert_array_equal(num_[0][0], array([1.0, 2.0], dtype=float))

def testTupleAllTypes(self):
"""Test tuple of a single value for all data types."""
for dtype in [int, int8, int16, int32, int64, float, float16, float32, float64, float128]:
num = (dtype(1),)
num_ = _cleanPart(num)
assert isinstance(num_, list)
assert np.all([isinstance(part, list) for part in num_])
np.testing.assert_array_equal(num_[0][0], array([1.0], dtype=float))

def testTupleAllTypes2(self):
"""Test tuple of a single value for all data types."""
for dtype in [int, int8, int16, int32, int64, float, float16, float32, float64, float128]:
num = (dtype(1), dtype(2))
num_ = _cleanPart(num)
assert isinstance(num_, list)
assert np.all([isinstance(part, list) for part in num_])
np.testing.assert_array_equal(num_[0][0], array([1, 2], dtype=float))

def testListListListInt(self):
""" Test an int in a list of a list of a list."""
num = [[[1]]]
num_ = _cleanPart(num)
assert isinstance(num_, list)
assert np.all([isinstance(part, list) for part in num_])
np.testing.assert_array_equal(num_[0][0], array([1.0], dtype=float))

def testListListListFloat(self):
""" Test a float in a list of a list of a list."""
num = [[[1.0]]]
num_ = _cleanPart(num)
assert isinstance(num_, list)
assert np.all([isinstance(part, list) for part in num_])
np.testing.assert_array_equal(num_[0][0], array([1.0], dtype=float))

def testListListListInts(self):
"""Test 2 lists of ints in a list in a list."""
num = [[[1,1],[2,2]]]
num_ = _cleanPart(num)

assert isinstance(num_, list)
assert np.all([isinstance(part, list) for part in num_])
np.testing.assert_array_equal(num_[0][0], array([1.0, 1.0], dtype=float))
np.testing.assert_array_equal(num_[0][1], array([2.0, 2.0], dtype=float))

def testListListListFloats(self):
"""Test 2 lists of ints in a list in a list."""

@roryyorke roryyorke Mar 17, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

floats, not ints?

num = [[[1.0,1.0],[2.0,2.0]]]
num_ = _cleanPart(num)

assert isinstance(num_, list)
assert np.all([isinstance(part, list) for part in num_])
np.testing.assert_array_equal(num_[0][0], array([1.0, 1.0], dtype=float))
np.testing.assert_array_equal(num_[0][1], array([2.0, 2.0], dtype=float))

def testListListArray(self):
"""List of list of numpy arrays for all valid types."""
for dtype in int, int8, int16, int32, int64, float, float16, float32, float64, float128:
num = [[array([1,1], dtype=dtype),array([2,2], dtype=dtype)]]
num_ = _cleanPart(num)

assert isinstance(num_, list)
assert np.all([isinstance(part, list) for part in num_])
np.testing.assert_array_equal(num_[0][0], array([1.0, 1.0], dtype=float))
np.testing.assert_array_equal(num_[0][1], array([2.0, 2.0], dtype=float))

def testTupleListArray(self):
"""Tuple of list of numpy arrays for all valid types."""
for dtype in int, int8, int16, int32, int64, float, float16, float32, float64, float128:
num = ([array([1,1], dtype=dtype),array([2,2], dtype=dtype)],)
num_ = _cleanPart(num)

assert isinstance(num_, list)
assert np.all([isinstance(part, list) for part in num_])
np.testing.assert_array_equal(num_[0][0], array([1.0, 1.0], dtype=float))
np.testing.assert_array_equal(num_[0][1], array([2.0, 2.0], dtype=float))

def testListTupleArray(self):
"""List of tuple of numpy array for all valid types."""
for dtype in int, int8, int16, int32, int64, float, float16, float32, float64, float128:
num = [(array([1,1], dtype=dtype),array([2,2], dtype=dtype))]
num_ = _cleanPart(num)

assert isinstance(num_, list)
assert np.all([isinstance(part, list) for part in num_])
np.testing.assert_array_equal(num_[0][0], array([1.0, 1.0], dtype=float))
np.testing.assert_array_equal(num_[0][1], array([2.0, 2.0], dtype=float))

def testTupleTuplesArrays(self):
"""Tuple of tuples of numpy arrays for all valid types."""
for dtype in int, int8, int16, int32, int64, float, float16, float32, float64, float128:
num = ((array([1,1], dtype=dtype),array([2,2], dtype=dtype)),
(array([3,4], dtype=dtype),array([4,4], dtype=dtype)))
num_ = _cleanPart(num)

assert isinstance(num_, list)
assert np.all([isinstance(part, list) for part in num_])
np.testing.assert_array_equal(num_[0][0], array([1.0, 1.0], dtype=float))
np.testing.assert_array_equal(num_[0][1], array([2.0, 2.0], dtype=float))

def testListTuplesArrays(self):
"""List of tuples of numpy arrays for all valid types."""
for dtype in int, int8, int16, int32, int64, float, float16, float32, float64, float128:
num = [(array([1,1], dtype=dtype),array([2,2], dtype=dtype)),
(array([3,4], dtype=dtype),array([4,4], dtype=dtype))]
num_ = _cleanPart(num)

assert isinstance(num_, list)
assert np.all([isinstance(part, list) for part in num_])
np.testing.assert_array_equal(num_[0][0], array([1.0, 1.0], dtype=float))
np.testing.assert_array_equal(num_[0][1], array([2.0, 2.0], dtype=float))

def testListListArrays(self):
"""List of list of numpy arrays for all valid types."""
for dtype in int, int8, int16, int32, int64, float, float16, float32, float64, float128:
num = [[array([1,1], dtype=dtype),array([2,2], dtype=dtype)],
[array([3,3], dtype=dtype),array([4,4], dtype=dtype)]]
num_ = _cleanPart(num)

assert len(num_) == 2
assert np.all([isinstance(part, list) for part in num_])
assert np.all([len(part) == 2 for part in num_])
np.testing.assert_array_equal(num_[0][0], array([1.0, 1.0], dtype=float))
np.testing.assert_array_equal(num_[0][1], array([2.0, 2.0], dtype=float))
np.testing.assert_array_equal(num_[1][0], array([3.0, 3.0], dtype=float))
np.testing.assert_array_equal(num_[1][1], array([4.0, 4.0], dtype=float))

def suite():
return unittest.TestLoader().loadTestsFromTestCase(TestXferFcnInput)

if __name__ == "__main__":
unittest.main()
93 changes: 50 additions & 43 deletions control/xferfcn.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
# Python 3 compatibility (needs to go here)
from __future__ import print_function
from __future__ import division
from __future__ import absolute_import

@roryyorke roryyorke Mar 17, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Had to look this up. It appears to be necessary for Python 2.5 - do we support that? I didn't quite understand what the related PEP was getting at, but it's not obvious to me that your changes especially need the features it enables anyway.

@dapperfu dapperfu Jun 26, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

True, I just used my boilerplate "I need to make something Python 2/3 compatible" code. It can probably be removed.


"""Copyright (c) 2010 by California Institute of Technology
All rights reserved.
Expand Down Expand Up @@ -55,6 +56,13 @@
from numpy import angle, any, array, empty, finfo, insert, ndarray, ones, \
polyadd, polymul, polyval, roots, sort, sqrt, zeros, squeeze, exp, pi, \
where, delete, real, poly, poly1d

from numpy import int, int8, int16, int32, int64
from numpy import float, float16, float32, float64, float128
from numpy import complex, complex64, complex128, complex256

from copy import deepcopy

import numpy as np
from scipy.signal import lti, tf2zpk, zpk2tf, cont2discrete
from copy import deepcopy
Expand Down Expand Up @@ -96,7 +104,7 @@ def __init__(self, *args):
where sys is a TransferFunction object (continuous or discrete).

"""

args = deepcopy(args)
if len(args) == 2:
# The user provided a numerator and a denominator.
(num, den) = args
Expand All @@ -120,48 +128,8 @@ def __init__(self, *args):
raise ValueError("Needs 1, 2 or 3 arguments; received %i."
% len(args))

# Make num and den into lists of lists of arrays, if necessary.
# Beware: this is a shallow copy! This should be okay,
# but be careful.
data = [num, den]
for i in range(len(data)):
# Check for a scalar (including 0d ndarray)
if (isinstance(data[i], (int, float, complex)) or
(isinstance(data[i], ndarray) and data[i].ndim == 0)):
# Convert scalar to list of list of array.
if (isinstance(data[i], int)):
# Convert integers to floats at this point
data[i] = [[array([data[i]], dtype=float)]]
else:
data[i] = [[array([data[i]])]]
elif (isinstance(data[i], (list, tuple, ndarray)) and
isinstance(data[i][0], (int, float, complex))):
# Convert array to list of list of array.
if (isinstance(data[i][0], int)):
# Convert integers to floats at this point
#! Not sure this covers all cases correctly
data[i] = [[array(data[i], dtype=float)]]
else:
data[i] = [[array(data[i])]]
elif (isinstance(data[i], list) and
isinstance(data[i][0], list) and
isinstance(data[i][0][0], (list, tuple, ndarray)) and
isinstance(data[i][0][0][0], (int, float, complex))):
# We might already have the right format. Convert the
# coefficient vectors to arrays, if necessary.
for j in range(len(data[i])):
for k in range(len(data[i][j])):
if (isinstance(data[i][j][k], int)):
data[i][j][k] = array(data[i][j][k], dtype=float)
else:
data[i][j][k] = array(data[i][j][k])
else:
# If the user passed in anything else, then it's unclear what
# the meaning is.
raise TypeError("The numerator and denominator inputs must be \
scalars or vectors (for\nSISO), or lists of lists of vectors (for SISO or \
MIMO).")
[num, den] = data
num = _cleanPart(num)
den = _cleanPart(den)

inputs = len(num[0])
outputs = len(num)
Expand Down Expand Up @@ -1349,3 +1317,42 @@ def tfdata(sys):
tf = _convertToTransferFunction(sys)

return (tf.num, tf.den)

def _cleanPart(data):
'''
Return a valid, cleaned up numerator or denominator
for the TransferFunction class.

Parameters:
data: numerator or denominator of a transfer function.

Returns:
data: correctly formatted transfer function part.

@roryyorke roryyorke Mar 17, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

you could be a little more specific: return data as a list of lists of ndarrays of type float.

;
'''
valid_types = (int, int8, int16, int32, int64,
float, float16, float32, float64, float128)
valid_collection = (list, tuple, ndarray)

if (isinstance(data, valid_types) or

@roryyorke roryyorke Mar 17, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I was going to suggest using np.isreal instead of isinstance(..,valid_types), but then I found np.isreal(file('/dev/null')) returns True (oops!).

(isinstance(data, ndarray) and data.ndim == 0)):
return [[array([data], dtype=float)]]
elif (isinstance(data, valid_collection) and
all([isinstance(d, valid_types) for d in data])):
return [[array(data, dtype=float)]]
elif (isinstance(data, (list, tuple)) and
isinstance(data[0], (list, tuple)) and
(isinstance(data[0][0], valid_collection) and
all([isinstance(d, valid_types) for d in data[0][0]]))):
data = list(data)
for j in range(len(data)):
data[j] = list(data[j])
for k in range(len(data[j])):
data[j][k] = array(data[j][k], dtype=float)
return data
else:
# If the user passed in anything else, then it's unclear what
# the meaning is.
raise TypeError("The numerator and denominator inputs must be \
scalars or vectors (for\nSISO), or lists of lists of vectors (for SISO or \
MIMO).")