narwhals icon indicating copy to clipboard operation
narwhals copied to clipboard

feat: Add `testing.assert_frame_equal`

Open FBruzzesi opened this issue 2 months ago â€Ē 4 comments

What type of PR is this? (check all applicable)

  • [ ] ðŸ’ū Refactor
  • [x] âœĻ Feature
  • [ ] 🐛 Bug Fix
  • [ ] 🔧 Optimization
  • [ ] 📝 Documentation
  • [ ] ✅ Test
  • [ ] ðŸģ Other

Related issues

  • Closes #3129
  • Natural follow up on #2983
  • Possible follow up #3221

Checklist

  • [x] Code follows style guide (ruff)
  • [x] Tests added
  • [x] Documented the changes

If you have comments or can explain your changes, please do so below

50%+ of the changes are docstrings and tests

FBruzzesi avatar Oct 18 '25 10:10 FBruzzesi

@FBruzzesi you might be able to adapt this test from (https://github.com/vega/altair/pull/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),
)
shape: (4, 2)
┌───────────────────────────┮───────┐
│ datetime                  ┆ value │
│ ---                       ┆ ---   │
│ datetime[ξs, US/Mountain] ┆ i64   │
╞═══════════════════════════╩═══════╡
│ 2023-11-05 00:00:00 MDT   ┆ 10    │
│ 2023-11-05 01:00:00 MDT   ┆ 20    │
│ 2023-11-05 01:00:00 MST   ┆ 30    │
│ 2023-11-05 02:00:00 MST   ┆ 40    │
└───────────────────────────â”ī───────┘

dangotbanned avatar Nov 24 '25 10:11 dangotbanned

Hey @dangotbanned 👋🏞 I am missing some context for the https://github.com/narwhals-dev/narwhals/pull/3220#issuecomment-3570084015

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 assert_series_equal), which is then dispatched with the usual narwhals mechanism

is_not_equal_mask = left != right
if is_not_equal_mask.any(): ...

FBruzzesi avatar Nov 25 '25 16:11 FBruzzesi

Hey @dangotbanned 👋🏞 I am missing some context for the #3220 (comment)

What would you like to see achieved?

Oof my bad, I was supposed to have replied on the (https://github.com/narwhals-dev/narwhals/pull/3220#discussion_r2442074358) thread 😅

Looking for help in finding edge cases here 😇

Maybe I'm naive 😉, but I hadn't come across that timezone before - so I thought it might help?

dangotbanned avatar Nov 25 '25 16:11 dangotbanned

[!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 tests/frame/)

FBruzzesi avatar Nov 27 '25 20:11 FBruzzesi