Conversation
|
Note Unrelated TPCH failure, see #3450 |
I used a different fix because we have an `__eq__` and `__hash__` override https://docs.astral.sh/ruff/rules/single-item-membership-test/
| assert dtype == nw.List(nw.List(nw.Int64)) | ||
| assert dtype == nw.List | ||
| assert dtype != nw.List(nw.List(nw.Float32)) | ||
| assert dtype in {nw.List(nw.List(nw.Int64))} |
There was a problem hiding this comment.
are we sure about changing this? wouldn't we still want to test that assert dtype in {nw.List(nw.List(nw.Int64))} passes?
like this it feels like we're testing implementation details, it may be better to just noqa it?
There was a problem hiding this comment.
How about a function (with whatever in it), that has a name more fitting to the test?
E.g.
- assert dtype in {nw.List(nw.List(nw.Int64))}
+ assert_hash_equals(dtype, nw.List(nw.List(nw.Int64)))I actually applied the auto-fix first, before realizing it changed the thing we test.
That seems like a pretty easy mistake to make and I might not have caught it without knowing the implementation detail 😅
Edit:
How easy it is to make that mistake?
Well I managed to do it in the same commit by only getting the first one right 🤦♂️
Lines 534 to 537 in b144f1e
There was a problem hiding this comment.
Hopefully this'll do 🙂 (6d5e3d5)
> assert_equal_hash(dtype, nw.Enum(["a", "b", "c"]))
E AssertionError: inputs do not compare equal by `__hash__`
E [left]: Enum(categories=['a', 'b'])
E [right]: Enum(categories=['a', 'b', 'c'])
tests/dtypes/dtypes_test.py:548: AssertionErrorBased on what's in narwhals.tests
| def _with_binary( | ||
| self, call: Callable[[dx.Series, Any], dx.Series], other: Any | ||
| ) -> Self: | ||
| return self._with_callable(lambda expr, other: call(expr, other), other=other) |
2 participants
Description
See fancy blog post for changes https://astral.sh/blog/ruff-v0.15.0
How does that impact us?
See (chore(ruff): Ignore new issues) and (style(ruff): Run new style guide)
Related issues