fix: fix type __get_item__#1958
Conversation
There was a problem hiding this comment.
not sure if changing this should be allowed, but in theory we are just expanding the allowed types (adding np.array that is suppoerted anyway)
🤔
There was a problem hiding this comment.
not sure if changing this should be allowed, but in theory we are just expanding the allowed types (adding
np.arraythat is suppoerted anyway) 🤔
The polars logic for this is very complex, are you sure narwhals already supports np.ndarray?
https://github.com/pola-rs/polars/blob/13186ba9e45a05ec7ced27fa18f304a9e6c57fa2/py-polars/polars/dataframe/frame.py#L1219-L1380
https://github.com/pola-rs/polars/blob/13186ba9e45a05ec7ced27fa18f304a9e6c57fa2/py-polars/polars/_utils/getitem.py
https://github.com/pola-rs/polars/blob/13186ba9e45a05ec7ced27fa18f304a9e6c57fa2/py-polars/polars/_typing.py#L281-L303
https://github.com/pola-rs/polars/blob/13186ba9e45a05ec7ced27fa18f304a9e6c57fa2/py-polars/polars/_utils/various.py#L92-L161
There was a problem hiding this comment.
Yup, totally allowed 😄 adding types should still be backwards-compatible
|
@EdAbati I think the issue you're running into is that import numpy as np
from collections.abc import Sequence
>>> isinstance(np.ndarray((2,2)), Sequence)
FalseEditI might have spoke too soon, I'm getting the same failures on (https://github.com/narwhals-dev/narwhals/actions/runs/13200872617/job/36852325981?pr=1955) |
MarcoGorelli
left a comment
MarcoGorelli
left a comment
There was a problem hiding this comment.
thanks @EdAbati , and Dan for reviewing!
There was a problem hiding this comment.
Yup, totally allowed 😄 adding types should still be backwards-compatible
3 participants
What type of PR is this? (check all applicable)
Related issues
Related to the
mypydiscussion on discord. I found few errors that should be better tackled in separate PRs :)Checklist
If you have comments or can explain your changes, please do so below