-
Notifications
You must be signed in to change notification settings - Fork 124
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add ConstructorEager type #1091
feat: add ConstructorEager type #1091
Conversation
Getting some issues with type checking.
Should I also add EDIT: |
Solved |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @lucianosrp ! just a comment, but I think the rest is fine
narwhals/typing.py
Outdated
def __getitem__(self, *args: Any, **kwargs: Any) -> Any: ... | ||
|
||
class DataFrameLike(Protocol): | ||
def __dataframe__(self, *args: Any, **kwargs: Any) -> Any: ... | ||
def __getitem__(self, *args: Any, **kwargs: Any) -> Any: ... | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should be modifying this
I think you can rewrite the test as follows:
|
This has been stale for a while. I'll try to make it right this weekend π₯ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
awesome, thanks @lucianosrp !
8e27021
to
15e2ef6
Compare
Many thanks @MarcoGorelli ! I will merge it nowπ |
What type of PR is this? (check all applicable)
Related issues
constructor_eager
asConstructorEager
instead ofConstructor
Β #1013Checklist
If you have comments or can explain your changes, please do so below.