perf: add_rows follow-up, also check for empty table (no rows) #607
Conversation
|
@Marsmaennchen221 Here is the check for amount of rows equals 0 |
| @@ -1059,7 +1059,7 @@ def add_rows(self, rows: list[Row] | Table) -> Table: | |||
| if isinstance(rows, Table): | |||
| if rows.number_of_rows == 0: | |||
| return self | |||
| if self.number_of_columns == 0: | |||
| if self.number_of_columns == 0 or self.number_of_rows == 0: | |||
| return rows | |||
| different_column_names = set(self.column_names) - set(rows.column_names) | |||
There was a problem hiding this comment.
Not something that must be fixed in this PR (and not by you in general, since we do this wrong a few times):
Don't we need to compute the symmetric difference here?
There was a problem hiding this comment.
Symmetric difference seems correct
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #607 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 53 53
Lines 3208 3208
=========================================
Hits 3208 3208 ☔ View full report in Codecov by Sentry. |
| @@ -1057,9 +1057,9 @@ def add_rows(self, rows: list[Row] | Table) -> Table: | |||
| 2 5 6 | |||
| """ | |||
| if isinstance(rows, Table): | |||
| if rows.number_of_rows == 0: | |||
| if rows.number_of_columns == 0 or rows.number_of_rows == 0: | |||
There was a problem hiding this comment.
The second check is not really necessary since number_of_colums == 0 => number_of_rows == 0
| return self | ||
| if self.number_of_columns == 0: | ||
| if self.number_of_columns == 0 or self.number_of_rows == 0: |
There was a problem hiding this comment.
We should check after comparing columns to check for different schemas
There was a problem hiding this comment.
@Marsmaennchen221 But is the schema relevant, if there is nothing changed either way?
|
See #608 with an added test |
|
Superseded by #608 |
See #606