-
Notifications
You must be signed in to change notification settings - Fork 77
Assert equal for tables and collections #1328
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
Conversation
061aaf5
to
16ee033
Compare
@petrelharp as JK is away can I get a review here? |
@Mergifyio rebase |
@Mergifyio rebase |
Command
|
16ee033
to
687d962
Compare
Coverage isn't updating for some reason. |
687d962
to
72d9411
Compare
Codecov Report
@@ Coverage Diff @@
## main #1328 +/- ##
==========================================
- Coverage 93.84% 93.84% -0.01%
==========================================
Files 26 26
Lines 22273 22335 +62
Branches 1029 1053 +24
==========================================
+ Hits 20903 20961 +58
- Misses 1339 1340 +1
- Partials 31 34 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
||
def test_equal(self, table1, test_rows): | ||
table2 = self.table_class() | ||
for row in test_rows[:5]: |
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.
isn't this depending on the number of rows being 5? e.g., shouldn't you replace 5
with table1.num_rows
?
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.
The tables have a fix number of rows (10).
|
||
def test_num_rows(self, table1, test_rows): | ||
table2 = self.table_class() | ||
for row in test_rows[:4]: |
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.
4
-> table1.num_rows - 1
?
table2.add_row(**row) | ||
with pytest.raises( | ||
AssertionError, | ||
match=f"{type(table1).__name__} number of rows differ: self=5 other=4", |
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.
oh I see, many things are depending on the number 5. perhaps it's OK.
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.
Looks great! Thanks!!
|
72d9411
to
0ef4d4f
Compare
WIP - Tables are done, Table Collection has a few options that I'm working on.
Fixes #1076