-
Notifications
You must be signed in to change notification settings - Fork 282
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
New tests for formats, predicates, tables and utils #562
Conversation
tests/test_formats.py
Outdated
fmtc = ds.default_formatter.format_column | ||
|
||
description_list = ["A data scientist is someone who creates programming code and combines it with statistical knowledge to create insights from data."] | ||
pad = fmtc("Carreers", description_list) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very minor: "Careers"
tests/test_util.py
Outdated
@@ -78,7 +81,7 @@ def test_minimize_smooth(): | |||
|
|||
|
|||
def test_minimize_array(): | |||
assert _round_eq(2, ds.minimize(lambda x: (x[0]-2)**2, [0], array=True)) | |||
assert _round_eq(2, ds.minimize(lambda x: (x[0]-2)**2, [0], array=True, log=print)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you help me understand the purpose of this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
tests/test_util.py
Outdated
@@ -96,3 +99,14 @@ def test_proportions_from_distribution(): | |||
uniform = u.column(1) | |||
assert len(uniform) == 50 and _round_eq(1, sum(uniform)) | |||
assert [x in (0, 0.5, 1) for x in ds.sample_proportions(2, ds.make_array(.2, .3, .5))] | |||
|
|||
def test_is_non_string_iterable(): | |||
class Sequece_Class(collections.abc.Sequence): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor: Sequence_Class
tests/test_util.py
Outdated
def __len__(self): | ||
return len(self._args) | ||
|
||
assert ds.is_non_string_iterable(Sequece_Class()) == True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sequence_Class
tests/test_util.py
Outdated
def __len__(self): | ||
return len(self._args) | ||
|
||
assert ds.is_non_string_iterable(Sequece_Class()) == True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be useful to test on a list and array as well.
tests/test_tables.py
Outdated
@@ -1900,4 +1900,4 @@ def test_read_table(): | |||
assert isinstance(Table().read_table("tests/us-unemployment.csv"), Table) | |||
assert isinstance(Table().read_table("tests/us-unemployment-copy"), Table) | |||
assert isinstance(Table().read_table("tests/us-unemployment.txt"), Table) | |||
assert isinstance(Table().read_table("https://raw.githubusercontent.com/data-8/textbook/main/assets/data/deflategate.csv"), Table) | |||
assert isinstance(Table().read_table("https://raw.githubusercontent.com/data-8/textbook/main/assets/data/deflategate.csv"), Table) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we please not remove the extra line at the end?
tests/test_util.py
Outdated
@@ -78,7 +81,7 @@ def test_minimize_smooth(): | |||
|
|||
|
|||
def test_minimize_array(): | |||
assert _round_eq(2, ds.minimize(lambda x: (x[0]-2)**2, [0], array=True)) | |||
assert _round_eq(2, ds.minimize(lambda x: (x[0]-2)**2, [0], array=True, log=print)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
@davidwagner I've fixed this PR up, please go ahead and re-review when you get some time :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes made. LGTM.
Changes proposed:
New tests for the features already developed. Formats and Predicates with 100% coverage.