feat: Disallow casting temporal to numeric

[FBruzzesi]

Description

It aligns with the decision of not following polars in supertyping between temporal and numeric in #3396.

Questions/Observations:

1. What should we do in the case of a polars expr?

2. Let me know if we want to keep stable also V2, or none at all and do the check for all versions

   • Following Marco's comment in the live stream, I removed the check for V1 in b13135f

3. For lazy backends, you will see an "extra" dtype.is_numeric(), this is to avoid having to trigger a collect_schema() if there is no need for it

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

• 💾 Refactor
• ✨ Feature
• 🐛 Bug Fix
• 🔧 Optimization
• 📝 Documentation
• ✅ Test
• 🐳 Other

Related issues

• Closes disallow casting temporal to integer #121 well this is a ooold issue!

Checklist

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

[FBruzzesi]

TIL: polars allows also casting to Float, not only to Integer

[FBruzzesi]

CI failure is unrelated?!

Test was failing due to collect_schema in sqlframe with null only, hence the type is not defined. Because of this, you will see a suppress(Exception) context

[dangotbanned]

@FBruzzesi I know this PR isn't active, but I've been reading through marimo for (#3721) and thought of it

I think this would be pretty painful for them here:

• https://github.com/marimo-team/marimo/blob/c3e14e30082c397d2a560e794cfb718f461c169a/marimo/_plugins/ui/_impl/tables/narwhals_table.py

In short, they kinda need to be able to do this because pyarrow apparently doesn't support as much on temporal 😔

[FBruzzesi]

Hey @dangotbanned - which particular bit concerns you? The only cast to numeric in such file are from either decimal or types that are already numerical

[dangotbanned]

Hey @dangotbanned - which particular bit concerns you? The only cast to numeric ... or types that are already numerical

wow 🤦‍♂️ wow 🤦‍♂️ wow 🤦‍♂️

Sorry @FBruzzesi false alarm, JS is rubbing off on me:

Context

narwhals/src/narwhals/_plan/altair/expression.py

Lines 302 to 318 in a5c38b7

	_TEMPORAL_TIME_UNIT: FnMap[SingleTimeUnit_T] = {
	temporal.Year: "year",
	# has no equivalent
	# - "quarter" -> (Q1-Q4) polars has is without the prefix
	# - "month" -> (Jan-Dec)
	temporal.Day: "date",
	# has no equivalent
	# - "week" -> (W01-W52)
	# - "day" -> (Sunday-Saturday)
	temporal.OrdinalDay: "dayofyear",
	# NOTE: These aren't quite equivalent (vega -> Time, polars -> Int),
	# but for the purpose of visualization, you likely want the richer type
	temporal.Hour: "hours",
	temporal.Minute: "minutes",
	temporal.Second: "seconds",
	temporal.Millisecond: "milliseconds",
	}

The right-hand-side are vega expressions,.
There "hours", "minutes", "seconds" and "milliseconds" all result in what we'd call Time (which is "understandably" represented as Date 😭)

My mistake

So I connected the wrong dot here:

        if dtype == nw.Time:
            # Convert to timestamp in ms
            col_in_ms = (
                col.dt.hour().cast(nw.Int64) * 3600000
#                             ^^^^

Maybe they did too?
The only other reason I could see is to avoid an overflow issue, but I would've assumed both polars and pyarrow would promote the type if it got too big after multiplication?