feat: Add testing.assert_frame_equal
#3220
Conversation
| # TODO(FBruzzesi): Do we even this? What should we check? | ||
| # The best way could be to migrate away from `assert_equal_data` in the test suite | ||
| def test_values_mismatch(constructor: Constructor) -> None: ... |
There was a problem hiding this comment.
Looking for help in finding edge cases here π
narwhals/testing/asserts/utils.py
Outdated
| DataFramesDetail: TypeAlias = ( | ||
| Literal[ | ||
| "columns are not in the same order", | ||
| "dtypes do not match", | ||
| "height (row count) mismatch", | ||
| "implementation mismatch", | ||
| ] | ||
| | str | ||
| # NOTE: `| str` makes the literals above redundant, but they still show | ||
| # up when typing as autocompletion. | ||
| # The reason to have `str` is due to the fact that other details are dynamic | ||
| # and depend upon which columns lead to the assertion error. |
There was a problem hiding this comment.
@dangotbanned do you see a better way of doing this?
There was a problem hiding this comment.
My first thought would be to keep the *Detail aliases for static portion of the message.
I haven't looked through to see how the dynamic stuff works yet - particularly where it is sourced and where it gets inserted - but if you had these things separated into 2 parameters then it might work?
E.g. if you added another parameter with not_thing: str = "", that could just get skipped when empty?
There was a problem hiding this comment.
This example uses None and adds line breaks when you have content
|
@FBruzzesi you might be able to adapt this test from (vega/altair#3922) into something for timezones? At least the source data may give you something to compare π import polars as pl
start = pl.datetime(2023, 11, 5, time_zone="US/Mountain")
pl.select(
datetime=pl.datetime_range(start, start.dt.offset_by("3h"), "1h"),
value=pl.int_range(10, 50, 10),
) |
|
Hey @dangotbanned ππΌ I am missing some context for the #3220 (comment) What would you like to see achieved? If it's dataframe with one or more datetime with timezones colujmns, then those checks are delegated to the series equality π (via is_not_equal_mask = left != right
if is_not_equal_mask.any(): ... |
Oof my bad, I was supposed to have replied on the (#3220 (comment)) thread π
Maybe I'm naive π, but I hadn't come across that timezone before - so I thought it might help? |
|
Warning This proposal might end up be a bit overwhelming in one go The best way to test this implementation would be to start rolling it out as proposed in #3221 - I can branch out from here and start doing that, maybe on a specific subset of tests (say |
What type of PR is this? (check all applicable)
Related issues
assert_frame_equalΒ #3129assert_series_equalΒ #2983assert_equal_datatoassert_frame_equalΒ #3221Checklist
If you have comments or can explain your changes, please do so below
50%+ of the changes are docstrings and tests