enh: Start narwhals.stable.v2 and enforce deprecations#2814
enh: Start narwhals.stable.v2 and enforce deprecations#2814MarcoGorelli merged 55 commits intonarwhals-dev:mainfrom
narwhals.stable.v2 and enforce deprecations#2814Conversation
narwhals.stable.v2 and enforce deprecations
| return self._with_orderable_filtration( | ||
| lambda plx: self._to_compliant_expr(plx).gather_every(n=n, offset=offset) | ||
| lambda plx: self._to_compliant_expr(plx).gather_every(n=n, offset=offset) # type: ignore[attr-defined] |
There was a problem hiding this comment.
Hey @MarcoGorelli 👋
IIUC, would the motivation for removing deprecated members from Compliant* be that extenders of narwhals aren't required to implement them?
If so, this is a tricky problem to solve 🤔
Note
TL;DR: I'd recommend keeping deprecations in the protocols
Typing
The main downside for us is that the loss of typing information makes it more likely that we unknowingly regress on the v1 promise in the future.
Despite the deprecation, we can clearly see which things are deprecated atm, and have some IDE help with:
New [attr-defined]s make me think that this link between layers is now (statically) murky?
If the goal were just to keep a type checker happy, then the two options are:
- Add
Compliant*V1protocols, use them in*V1narwhals-level and implement both in the backends that supportV1 - Keep deprecations in
Compliant*protocols, punt the issue for now
Runtime
This is the trickier one for me ...
Would a new internal backend be expected to support v1?
For example, in the daft PR, we can see the typing distinction between things that aren't yet and won't ever be implemented.
gather_every = not_implemented.deprecated(
"`LazyFrame.gather_every` is deprecated and will be removed in a future version."
)
join_asof = not_implemented()
tail = not_implemented.deprecated(
"`LazyFrame.tail` is deprecated and will be removed in a future version."
)
explode = not_implemented()At runtime we give the same message to users (I don't know why I made that decision (#2232 (comment)) 😅)
A new backend won't get a type checker yelling to add a not_implemented, which at runtime will mean we just get an AttributeError 😢
There was a problem hiding this comment.
we've got a file which specifically tests all the functionality from v1 (tests/v1_test.py), if we're removing the functionality then I think it looks a bit odd to keep it but marked as deprecated
Would a new internal backend be expected to support v1?
no, we only need to care about preserving functionality for code that's already been written
| class DataFrame(NwDataFrame[IntoDataFrameT]): | ||
| @inherit_doc(NwDataFrame) | ||
| def __init__(self, df: Any, *, level: Literal["full", "lazy", "interchange"]) -> None: | ||
| assert df._version is Version.V2 # noqa: S101 | ||
| super().__init__(df, level=level) |
There was a problem hiding this comment.
We can move level to v1-only can't we?
Or at least "interchange" I thought was on the chopping block?
There was a problem hiding this comment.
yes true, i just think it'd require a larger refactor
it's not user-facing though, so not necessarily pressing?
There was a problem hiding this comment.
Yeah that seems fair
How about in this PR, try removing v2.typing.DataFrameLike and adding some tests using a mock w/ __dataframe__?
I think making sure the typing on v2 rejects interchange only should be doable 🙂
We can punt the rest to another issue
|
Just tried v2 out in Bokeh, and it all passes as-is In Altair it requires some code changes because |


From the downstream tests, the only breakage is in:
native_namespaceinfrom_dictThat's...all, from what I can tell.
I've made a PR to them to update this: Nixtla/hierarchicalforecast#381
I don't want to be disruptive towards them, so perhaps we could take a bit of a middle-ground approach:
native_namespaceinfrom_dict, keep it around longer in the main Narwhals namespace at least until there's been a new hierarchicalforecasttodo:
from_native_test, separate out some tests to justv1, and see what we need to test forv2as wellAiming to merge this (and make a Narwhals 2.0 release) by August