ci: Improving `generate_random_versions`

[dangotbanned]

Description

I've been stung by a false sense of security from this guy utils/generate_random_versions.py:

• Most recent failure, which hadn't caught everything from these one's
• ci(expr-ir): Fix coverage/tests in ci #3622
• fix(expr-ir): Add compat for old polars #3617
• feat(expr-ir): Long-overdue sync #3611
• fix(expr-ir): Add more compat for old polars #3624

So I wanted to discuss ways we could improve the script.
It does catch things but often later than the PR that introduced the bug 😔

Idea 1

I think a lot of my pain would've been avoided if polars 1.0.0 or 1.1.0 had a greater chance of running.

That is only going to happen on ~9% of runs (2/23) right now, so maybe we could weight the choice towards these versions some more?

@FBruzzesi I hear you do the good maths, can we do any better than random.choice?

narwhals/utils/generate_random_versions.py

Line 55 in 9200736

polars_version = random.choice(POLARS_VERSION)

Idea ...

Help me out here

[FBruzzesi]

Not sure if I would agree with:

we could weight the choice towards these versions some more?

Technically should be easy to achieve I would argue for running the workflow more than once to generate multiple matrices of dependencies. If we think there is the need to always test a subset of version we can split it into two

[dangotbanned]

Technically should be easy to achieve I would argue for running the workflow more than once to generate multiple matrices of dependencies.

Ah, I missed out on some color I could add to how I got here 😅

Context

These are all the polars compat paths I've identified in (#2572):

Show narwhals/_plan/polars/compat

narwhals/narwhals/_plan/polars/compat.py

Lines 31 to 129 in 8d491b0

	# TODO @dangotbanned: Remove this dead code path on main, it is fixed at our minimum
	HAS_POLARS_ERROR: Final = BACKEND_VERSION >= (0, 20, 4)
	"""https://github.com/pola-rs/polars/pull/13615"""
	SERIES_SORT_SUPPORTS_NULLS_LAST: Final = BACKEND_VERSION >= (0, 20, 6)
	"""https://github.com/pola-rs/polars/pull/13794
	Prior this this version, `nulls_last` was [only available on `*Frame` and `Expr`].
	[only available on `*Frame` and `Expr`]: https://github.com/pola-rs/polars/issues/13788
	"""
	HAS_LEN: Final = BACKEND_VERSION >= (0, 20, 5)
	"""https://github.com/pola-rs/polars/pull/13719"""
	DUNDER_ARRAY_SUPPORTS_COPY: Final = BACKEND_VERSION >= (0, 20, 29)
	"""https://github.com/pola-rs/polars/pull/16401"""
	JOIN_OUTER_RENAMED_TO_FULL: Final = BACKEND_VERSION >= (0, 20, 29)
	"""https://github.com/pola-rs/polars/pull/16417"""
	SERIES_HAS_HAS_NULLS: Final = BACKEND_VERSION >= (0, 20, 30)
	"""https://github.com/pola-rs/polars/pull/16488"""
	NULLS_LAST_ACCEPTS_MULTIPLE: Final = BACKEND_VERSION >= (0, 20, 31)
	"""https://github.com/pola-rs/polars/pull/16639"""
	MELT_RENAMED_TO_UNPIVOT: Final = BACKEND_VERSION >= (1, 0)
	"""https://github.com/pola-rs/polars/pull/17095"""
	LAZYFRAME_HAS_COLLECT_SCHEMA: Final = BACKEND_VERSION >= (1, 0)
	CONSTRUCTOR_ACCEPTS_PYCAPSULE: Final = BACKEND_VERSION >= (1, 3)
	"""https://github.com/pola-rs/polars/pull/17693"""
	SERIES_HAS_FIRST_LAST: Final = BACKEND_VERSION >= (1, 10)
	"""https://github.com/pola-rs/polars/pull/19093"""
	SERIES_RFLOORDIV_HANDLES_ZERO: Final = BACKEND_VERSION >= (1, 10)
	"""https://github.com/pola-rs/polars/issues/19142
	Note:
	The bug impacts `__rmod__` as well, but didn't get fixed in narwhals?
	"""
	SERIES_RPOW_PRESERVES_NAME: Final = BACKEND_VERSION >= (1, 16, 1)
	"""https://github.com/pola-rs/polars/pull/20072"""
	IS_NAN_NUMERIC_PRESERVES_NULLS: Final = BACKEND_VERSION >= (1, 18)
	"""https://github.com/pola-rs/polars/pull/20386"""
	MIN_PERIODS_RENAMED_TO_MIN_SAMPLES: Final = BACKEND_VERSION >= (1, 21)
	"""https://github.com/pola-rs/polars/pull/20850"""
	HAS_LINEAR_SPACE: Final = BACKEND_VERSION >= (1, 21)
	"""https://github.com/pola-rs/polars/pull/20678"""
	PIVOT_SUPPORTS_ON_COLUMNS: Final = BACKEND_VERSION >= (1, 36)
	"""https://github.com/pola-rs/polars/pull/25016"""
	OVER_SUPPORTS_ORDER_BY: Final = BACKEND_VERSION >= (1, 0)
	"""https://github.com/pola-rs/polars/pull/16743
	Supports:
	pl.col("a").first().over("b", order_by="c")
	"""
	OVER_SUPPORTS_DESCENDING: Final = BACKEND_VERSION >= (1, 22)
	"""[#20919], [#20952]
	Supports:
	pl.col("a").first().over("b", order_by="c", descending=True)
	pl.col("a").first().over("b", order_by="c", descending=True, nulls_last=False)
	[#20919]: https://github.com/pola-rs/polars/pull/20919
	[#20952]: https://github.com/pola-rs/polars/pull/20952
	"""
	OVER_WITHOUT_PARTITION_BY: Final = BACKEND_VERSION >= (1, 30)
	"""https://github.com/pola-rs/polars/pull/22712
	**Use `over(pl.lit(1), order_by=...)` on older versions!**
	Supports:
	pl.col("a").first().over(order_by="c", descending=True, nulls_last=False)
	"""
	OVER_RESPECTS_NULLS_LAST: Final = BACKEND_VERSION >= (1, 39)
	"""https://github.com/pola-rs/polars/pull/26718
	Supports:
	pl.col("a").first().over(order_by="c", descending=True, nulls_last=True)
	"""

And these are some we have on main, which I'm either reusing or haven't implemented yet:

Show lots more

narwhals/src/narwhals/_polars/utils.py

Lines 52 to 55 in 93a9921

	SERIES_ACCEPTS_PD_INDEX: Final[bool] = BACKEND_VERSION >= (0, 20, 7)
	"""`pl.Series(values: pd.Index)` fixed in https://github.com/pola-rs/polars/pull/14087"""
	SERIES_RESPECTS_DTYPE: Final[bool] = BACKEND_VERSION >= (0, 20, 26)

narwhals/src/narwhals/_polars/utils.py

Line 61 in 93a9921

HAS_INT_128 = BACKEND_VERSION >= (1, 18, 0)

narwhals/src/narwhals/_polars/utils.py

Line 64 in 93a9921

FROM_DICTS_ACCEPTS_MAPPINGS: Final[bool] = BACKEND_VERSION >= (1, 30, 0)

narwhals/src/narwhals/_polars/utils.py

Line 67 in 93a9921

HAS_UINT_128 = BACKEND_VERSION >= (1, 34, 0)

narwhals/src/narwhals/_polars/utils.py

Line 234 in 93a9921

if BACKEND_VERSION >= (1,):

narwhals/src/narwhals/_polars/namespace.py

Lines 114 to 117 in 93a9921

	@requires.backend_version(
	(1, 0, 0), "Please use `col` for columns selection instead."
	)
	def nth(self, indices: Sequence[int]) -> PolarsExpr:

narwhals/src/narwhals/_polars/namespace.py

Lines 152 to 153 in 93a9921

	def mean_horizontal(self, *exprs: PolarsExpr) -> PolarsExpr:
	if self._backend_version < (0, 20, 8):

narwhals/src/narwhals/_polars/namespace.py

Line 169 in 93a9921

if self._backend_version < (0, 20, 6):

narwhals/src/narwhals/_polars/expr.py

Line 102 in 93a9921

name = "min_periods" if self._backend_version < (1, 21, 0) else "min_samples"

narwhals/src/narwhals/_polars/expr.py

Line 137 in 93a9921

if self._backend_version < (1,): # pragma: no cover

narwhals/src/narwhals/_polars/expr.py

Line 149 in 93a9921

if self._backend_version >= (1, 18):

narwhals/src/narwhals/_polars/expr.py

Line 159 in 93a9921

if self._backend_version < (1, 9):

narwhals/src/narwhals/_polars/expr.py

Line 168 in 93a9921

@requires.backend_version((1,))

narwhals/src/narwhals/_polars/expr.py

Line 178 in 93a9921

@requires.backend_version((1,))

narwhals/src/narwhals/_polars/expr.py

Line 222 in 93a9921

if pl_version < (1, 32)

narwhals/src/narwhals/_polars/expr.py

Line 225 in 93a9921

kwargs = {} if pl_version < (0, 20, 31) else {"returns_scalar": returns_scalar}

narwhals/src/narwhals/_polars/expr.py

Line 229 in 93a9921

@requires.backend_version((1,))

narwhals/src/narwhals/_polars/expr.py

Line 296 in 93a9921

if self._backend_version < (1, 10, 0):

narwhals/src/narwhals/_polars/expr.py

Line 427 in 93a9921

if BACKEND_VERSION < (1, 35):

narwhals/src/narwhals/_polars/expr.py

Line 439 in 93a9921

@requires.backend_version((0, 20, 5))

narwhals/src/narwhals/_polars/expr.py

Line 444 in 93a9921

if backend_version <= (1, 30, 0):

narwhals/src/narwhals/_polars/expr.py

Line 516 in 93a9921

if self.compliant._backend_version < (1, 16): # pragma: no cover

narwhals/src/narwhals/_polars/expr.py

Line 526 in 93a9921

if self.compliant._backend_version < (1, 28):

narwhals/src/narwhals/_polars/dataframe.py

Line 220 in 93a9921

"outer" if (self._backend_version < (0, 20, 29) and how == "full") else how

narwhals/src/narwhals/_polars/dataframe.py

Line 235 in 93a9921

if self._backend_version < (1, 0, 0):

narwhals/src/narwhals/_polars/dataframe.py

Line 306 in 93a9921

if context._implementation._backend_version() >= (1, 3):

narwhals/src/narwhals/_polars/dataframe.py

Line 459 in 93a9921

if self._backend_version > (0, 20, 30):

narwhals/src/narwhals/_polars/dataframe.py

Line 667 in 93a9921

self._backend_version < (1, 36)

narwhals/src/narwhals/_polars/dataframe.py

Line 762 in 93a9921

if self._backend_version < (1, 0, 0):

Summary

I was eyeballing these and they seem to skew towards 1.*.
But there's a lot that aren't covered in the narrow space of:

narwhals/utils/generate_random_versions.py

Lines 36 to 37 in 9200736

	"1.0.0",
	"1.1.0",

Compared to all of these below 1.0.0

narwhals/utils/generate_random_versions.py

Lines 15 to 35 in 9200736

	"0.20.4",
	"0.20.5",
	"0.20.6",
	"0.20.7",
	"0.20.8",
	"0.20.9",
	"0.20.10",
	"0.20.13",
	"0.20.14",
	"0.20.15",
	"0.20.16",
	"0.20.17",
	"0.20.18",
	"0.20.19",
	"0.20.21",
	"0.20.22",
	"0.20.23",
	"0.20.25",
	"0.20.26",
	"0.20.30",
	"0.20.31",

So on this point:

@FBruzzesi
running the workflow more than once to generate multiple matrices of dependencies.

This would improve our chances, but if we started at:

That is only going to happen on ~9% of runs (2/23) right now,

Then that option would (in theory) double the workflow runtime to improve the odds to 18% 😳

So my hope was that we could weight things towards versions that have more incompatibilities (that we know of), without increasing our CI runtime 🙏

[dangotbanned]

All the options

1. Keep things as-is
2. Run the workflow multiple times
3. Using something besides random.choice that allows us to control the distributions
4. Change the versions we test (e.g. skip some versions, maybe add some more beyond 1.1.0)
5. Something else?

[dangotbanned]

• https://github.com/narwhals-dev/narwhals/actions/runs/26403042128/job/77719743303#step:8:29

This one has been there for a while, but doesn't get hit often enough to get prioritized:

FAILED tests/frame/join_test.py::test_join_on_null_values[polars[eager]-semi-expected4] - ValueError: zip() argument 2 is shorter than argument 1
FAILED tests/frame/join_test.py::test_join_on_null_values[polars[lazy]-semi-expected4] - ValueError: zip() argument 2 is shorter than argument 1
FAILED tests/frame/join_test.py::test_join_on_null_values[polars[eager]-anti-expected5] - ValueError: zip() argument 2 is longer than argument 1
FAILED tests/frame/join_test.py::test_join_on_null_values[polars[lazy]-anti-expected5] - ValueError: zip() argument 2 is longer than argument 1

[dangotbanned]

I've just noticed that the gaps in POLARS_VERSION are releases that were yanked.
That is a good idea, but it should probably be documented somewhere too 🙂