Skip to content

Commit

Permalink
Address comments.
Browse files Browse the repository at this point in the history
  • Loading branch information
plypaul committed Jun 3, 2024
1 parent 0ece9ef commit 19beaba
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 13 deletions.
7 changes: 1 addition & 6 deletions metricflow/data_table/mf_table.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,6 @@ def column_values_iterator(self, column_index: int) -> Iterator[CellValue]:
return (row[column_index] for row in self.rows)

def _sorted_by_column_name(self) -> MetricFlowDataTable: # noqa: D102
# row_dict_by_row_index: Dict[int, Dict[str, CellType]] = defaultdict(dict)

new_rows: List[List[CellValue]] = [[] for _ in range(self.row_count)]
sorted_column_names = sorted(self.column_names)
for column_name in sorted_column_names:
Expand Down Expand Up @@ -142,10 +140,7 @@ def text_format(self, float_decimals: int = 2) -> str:
continue

if isinstance(cell_value, datetime.datetime):
if cell_value.time() == datetime.time.min:
str_row.append(cell_value.date().isoformat())
else:
str_row.append(cell_value.isoformat())
str_row.append(cell_value.isoformat())
continue

str_row.append(str(cell_value))
Expand Down
10 changes: 6 additions & 4 deletions tests_metricflow/sql/compare_data_table.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,14 +94,16 @@ def check_data_tables_are_equal(
This was migrated from an existing implementation based on `pandas` data_tables.
"""
if ignore_order:
expected_table = expected_table.sorted()
actual_table = actual_table.sorted()

if compare_column_names_using_lowercase:
expected_table = expected_table.with_lower_case_column_names()
actual_table = actual_table.with_lower_case_column_names()

# Sort after case changes since the order can change after a case change. e.g. underscore comes
# before lowercase.
if ignore_order:
expected_table = expected_table.sorted()
actual_table = actual_table.sorted()

if expected_table.column_names != actual_table.column_names:
raise ValueError(
mf_pformat_many(
Expand Down
9 changes: 6 additions & 3 deletions tests_metricflow/sql_clients/test_sql_client.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from __future__ import annotations

import logging
from typing import Set, Union
from typing import Optional, Set, Union

import pytest
from dbt_semantic_interfaces.test_utils import as_datetime
Expand All @@ -26,13 +26,16 @@ def _select_x_as_y(x: int = 1, y: str = "y") -> str:
return f"SELECT {x} AS {y}"


def _check_1col(df: MetricFlowDataTable, col: str = "y", vals: Set[Union[int, str]] = {1}) -> None:
def _check_1col(df: MetricFlowDataTable, col: str = "y", vals: Optional[Set[Union[int, str]]] = None) -> None:
"""Helper to check that 1 column has the same value and a case-insensitive matching name.
We lower-case the names due to snowflake's tendency to capitalize things. This isn't ideal but it'll do for now.
"""
if vals is None:
vals = {1}

assert df.column_count == 1
assert df.column_names == (col,)
assert tuple(column_name.lower() for column_name in df.column_names) == (col.lower(),)
assert set(df.column_values_iterator(0)) == vals


Expand Down

0 comments on commit 19beaba

Please sign in to comment.