warn or raise on explicit incompatible `TimeUnit`

[dangotbanned]

Related

• feat: Add Series.from_iterable #2933 (comment)
• feat: Add Series.from_iterable #2933 (comment)

Description

I was caught out in a test where for pandas<2, we silently ignore nw.Datetime(time_unit=...) and always use "ns".

It is understandable after reading up on the issue, and an upcoming breaking change in pandas.

However, we should be documenting what is happening - so that anyone trying to trace back an issue can find out the origin

[benrutter]

Sounds like a helpful warning! I'd love to put in a PR adding a warning in cases where pandas is ignoring a time_unit argument (I.e. if nw.Datetime(time_unit="s") was passed into pandas 1) as long as people agree that'd be helpful?

[dangotbanned]

Thanks @benrutter, yeah that's exactly what we're after!

Now is probably a good time as (#2969) merged

I think @MarcoGorelli's idea was a change here:

narwhals/narwhals/_pandas_like/utils.py

Lines 486 to 493 in 5ea6ee4

	if isinstance_or_issubclass(dtype, dtypes.Datetime):
	# Pandas does not support "ms" or "us" time units before version 2.0
	if is_pandas_or_modin(implementation) and PANDAS_VERSION < (
	2,
	): # pragma: no cover
	dt_time_unit = "ns"
	else:
	dt_time_unit = dtype.time_unit

So before we go straight to dt_time_unit = "ns" we would check if we had either:

dtypes.Datetime       # the class
dtypes.Datetime(...)  # an instance

The difference being that passing in class defaults to [us], but it should only warn if we asked for it

import narwhals as nw

>>> nw.Datetime.time_unit
'us'

>>> nw.Datetime("us").time_unit
'us'

Hope that all makes sense!

[benrutter]

Yeah, that all sounds good! I'll try and get a PR in today

[benrutter]

(sorry, didn't get round to it today! If someones super keen - I won't be offended if they go ahead and fix ahead of me, otherwise I'll put something in on Monday)