You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
@MarcoGorelli how would you feel about removing CompliantDataFrame.__getitem__ (and children) entirely?
At this stage - I think - we should have a fairly good idea of what method to call in EagerDataFrame.
But when we pass it down to EagerDataFrame.__getitem__ - all it knows is we have a tuple - then requiring more narrowing than I think we strictly have to 🤔
Description
I forgot about that particular comment.
But kept thinking about what an alternative might look like after discovering polars._utils.slice.
An easy way to avoid falling back into the patterns prior to (#2390) is to simply remove __getitem__ from the internal classes.
If we were to put slicing behind a namespace (e.g. CompliantSlice) then these methods could be entirely separate from the class(es) they're on now.
Important
This issue is not proposing changes to
nw.DataFrame
Original comment
https://github.com/narwhals-dev/narwhals/pull/2393/files#r2052510250
narwhals/narwhals/dataframe.py
Lines 934 to 938 in f47ad1f
Description
I forgot about that particular comment.
But kept thinking about what an alternative might look like after discovering
polars._utils.slice
.An easy way to avoid falling back into the patterns prior to (#2390) is to simply remove
__getitem__
from the internal classes.If we were to put slicing behind a namespace (e.g.
CompliantSlice
) then these methods could be entirely separate from the class(es) they're on now.narwhals/narwhals/_compliant/dataframe.py
Lines 401 to 417 in f47ad1f
So calls might look like this:
Where
_slice
is a property returning a type likenarwhals/narwhals/_arrow/namespace.py
Lines 41 to 43 in f47ad1f
Or the property initializes a class like:
narwhals/narwhals/_compliant/expr.py
Lines 849 to 851 in f47ad1f
Benefits
*Slice
classDataFrame
,Series
(feat: ImproveDataFrame.__getitem__
consistency #2393 (comment))not_implemented
😏The text was updated successfully, but these errors were encountered: