- 
                Notifications
    
You must be signed in to change notification settings  - Fork 170
 
          feat(typing): Make Implementation less opaque
          #3016
        
          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
Changes from 13 commits
2c36eea
              2350dfc
              fe80d52
              123dc2e
              54bfbe4
              cadcdf0
              5b2bc62
              14974bc
              685409c
              0f83e44
              49a10bd
              fd2b93e
              618ce8c
              0606a14
              37aaa69
              141b687
              cabedd4
              71c5163
              2b7945b
              c573cfd
              410b5bd
              e07cbc5
              c4bceed
              c8dbe07
              eaa43c1
              5ef8103
              f55cb3a
              811290c
              012c2bf
              b0694d0
              2a75529
              7d42972
              fd736c6
              5d2f54f
              05d4115
              87d4439
              bee6984
              1c68c68
              fcafec6
              7157bbd
              08d900c
              b2aaf0d
              635b5a8
              5049a2a
              4b78837
              29daf5e
              f6da9ce
              fe21d09
              a94a0f8
              884d135
              4ad081c
              791ecae
              a3bd3ac
              043b9d1
              6b59ed9
              919b22f
              5e2838e
              6bc2c47
              21800fe
              File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
| 
          
            
          
           | 
    @@ -71,8 +71,34 @@ | |||
| 
     | 
||||
| from narwhals._compliant import CompliantDataFrame, CompliantLazyFrame | ||||
| from narwhals._compliant.typing import CompliantExprAny, EagerNamespaceAny | ||||
| from narwhals._namespace import ( | ||||
| _CuDFDataFrame, | ||||
| _ModinDataFrame, | ||||
| _NativeDask, | ||||
| _NativeDuckDB, | ||||
| _NativePandasLikeDataFrame, | ||||
| _NativeSQLFrame, | ||||
| ) | ||||
| from narwhals._translate import IntoArrowTable | ||||
| from narwhals._typing import Dask, DuckDB, EagerAllowed, Ibis, IntoBackend, Polars | ||||
| from narwhals._typing import ( | ||||
| Dask, | ||||
| DuckDB, | ||||
| EagerAllowed, | ||||
| Ibis, | ||||
| IntoBackend, | ||||
| Polars, | ||||
| _ArrowImpl, | ||||
| _CudfImpl, | ||||
| _DaskImpl, | ||||
| _DuckDBImpl, | ||||
| _EagerAllowedImpl, | ||||
| _LazyAllowedImpl, | ||||
| _ModinImpl, | ||||
| _PandasImpl, | ||||
| _PandasLikeImpl, | ||||
| _PolarsImpl, | ||||
| _SQLFrameImpl, | ||||
| ) | ||||
| from narwhals.group_by import GroupBy, LazyGroupBy | ||||
| from narwhals.typing import ( | ||||
| AsofJoinStrategy, | ||||
| 
          
            
          
           | 
    @@ -104,10 +130,76 @@ | |||
| MultiIndexSelector: TypeAlias = "_MultiIndexSelector[Series[Any]]" | ||||
| 
     | 
||||
| 
     | 
||||
| class _ImplDescriptor: | ||||
| def __set_name__(self, owner: type[Any], name: str) -> None: | ||||
| self.__name__: str = name | ||||
| 
     | 
||||
| @overload | ||||
| def __get__( | ||||
| self, instance: DataFrame[pl.DataFrame] | LazyFrame[pl.LazyFrame], owner: Any | ||||
| ) -> _PolarsImpl: ... | ||||
| @overload | ||||
| def __get__(self, instance: BaseFrame[pd.DataFrame], owner: Any) -> _PandasImpl: ... | ||||
| @overload | ||||
| def __get__(self, instance: BaseFrame[_ModinDataFrame], owner: Any) -> _ModinImpl: ... | ||||
| 
     | 
||||
| @overload # oof, looks like these two need their names aligned π | ||||
| def __get__(self, instance: BaseFrame[_CuDFDataFrame], owner: Any) -> _CudfImpl: ... | ||||
| @overload | ||||
| def __get__( | ||||
| self, instance: BaseFrame[_NativePandasLikeDataFrame], owner: Any | ||||
| ) -> _PandasLikeImpl: ... | ||||
| @overload | ||||
| def __get__(self, instance: BaseFrame[pa.Table], owner: Any) -> _ArrowImpl: ... | ||||
| @overload | ||||
| def __get__( | ||||
| self, instance: BaseFrame[pl.DataFrame | pd.DataFrame | pa.Table], owner: Any | ||||
| ) -> _PolarsImpl | _PandasImpl | _ArrowImpl: ... | ||||
                
      
                  dangotbanned marked this conversation as resolved.
               
              
                Outdated
          
            Show resolved
            Hide resolved
         | 
||||
| @overload | ||||
| def __get__(self, instance: LazyFrame[_NativeDuckDB], owner: Any) -> _DuckDBImpl: ... | ||||
| @overload | ||||
| def __get__( | ||||
| self, instance: LazyFrame[_NativeSQLFrame], owner: Any | ||||
| ) -> _SQLFrameImpl: ... | ||||
| @overload | ||||
| def __get__(self, instance: LazyFrame[_NativeDask], owner: Any) -> _DaskImpl: ... | ||||
| @overload | ||||
| def __get__(self, instance: None, owner: Any) -> Self: ... | ||||
| @overload | ||||
| def __get__(self, instance: DataFrame[Any], owner: Any) -> _EagerAllowedImpl: ... | ||||
| @overload | ||||
| def __get__(self, instance: LazyFrame[Any], owner: Any) -> _LazyAllowedImpl: ... | ||||
                
      
                  dangotbanned marked this conversation as resolved.
               
              
                Outdated
          
            Show resolved
            Hide resolved
         | 
||||
| def __get__(self, instance: Any | None, owner: Any) -> Any: | ||||
| if instance is None: # pragma: no cover | ||||
| return self | ||||
| return instance._compliant_frame._implementation | ||||
| 
     | 
||||
| 
     | 
||||
| class BaseFrame(Generic[_FrameT]): | ||||
| _compliant_frame: Any | ||||
| _level: Literal["full", "lazy", "interchange"] | ||||
| 
     | 
||||
| implementation: _ImplDescriptor = _ImplDescriptor() | ||||
| """Return implementation of native frame. | ||||
| 
     | 
||||
| This can be useful when you need to use special-casing for features outside of | ||||
| Narwhals' scope - for example, when dealing with pandas' Period Dtype. | ||||
| 
         
      Comment on lines
    
      +112
     to 
      +113
    
   
  There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. True (and I know this was the description that was already here) - but this is not the only case in which  There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I agree tbf, I also thought the first line could do with some tweaking narwhals/narwhals/dataframe.py Line 113 in 635b5a8 
 If you suggest something, I'm 95% sure I'll accept it π There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we look at improving these docs in a follow-up? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes sure! Sorry I didn't mean it as a blocker  | 
||||
| 
     | 
||||
| Examples: | ||||
| >>> import narwhals as nw | ||||
| >>> import pandas as pd | ||||
| >>> df_native = pd.DataFrame({"a": [1, 2, 3]}) | ||||
| >>> df = nw.from_native(df_native) | ||||
| >>> df.implementation | ||||
| <Implementation.PANDAS: 'pandas'> | ||||
| >>> df.implementation.is_pandas() | ||||
| True | ||||
| >>> df.implementation.is_pandas_like() | ||||
| True | ||||
| >>> df.implementation.is_polars() | ||||
| False | ||||
| """ | ||||
| 
     | 
||||
| def __native_namespace__(self) -> ModuleType: | ||||
| return self._compliant_frame.__native_namespace__() # type: ignore[no-any-return] | ||||
| 
     | 
||||
| 
          
            
          
           | 
    @@ -660,29 +752,6 @@ def from_numpy( | |||
| ) | ||||
| raise ValueError(msg) | ||||
| 
     | 
||||
| @property | ||||
| def implementation(self) -> Implementation: | ||||
| """Return implementation of native frame. | ||||
| 
     | 
||||
| This can be useful when you need to use special-casing for features outside of | ||||
| Narwhals' scope - for example, when dealing with pandas' Period Dtype. | ||||
| 
     | 
||||
| Examples: | ||||
| >>> import narwhals as nw | ||||
| >>> import pandas as pd | ||||
| >>> df_native = pd.DataFrame({"a": [1, 2, 3]}) | ||||
| >>> df = nw.from_native(df_native) | ||||
| >>> df.implementation | ||||
| <Implementation.PANDAS: 'pandas'> | ||||
| >>> df.implementation.is_pandas() | ||||
| True | ||||
| >>> df.implementation.is_pandas_like() | ||||
| True | ||||
| >>> df.implementation.is_polars() | ||||
| False | ||||
| """ | ||||
| return self._compliant_frame._implementation | ||||
| 
     | 
||||
| def __len__(self) -> int: | ||||
| return self._compliant_frame.__len__() | ||||
| 
     | 
||||
| 
          
            
          
           | 
    @@ -2295,22 +2364,6 @@ def __init__(self, df: Any, *, level: Literal["full", "lazy", "interchange"]) -> | |||
| def __repr__(self) -> str: # pragma: no cover | ||||
| return generate_repr("Narwhals LazyFrame", self.to_native().__repr__()) | ||||
| 
     | 
||||
| @property | ||||
| def implementation(self) -> Implementation: | ||||
| """Return implementation of native frame. | ||||
| 
     | 
||||
| This can be useful when you need to use special-casing for features outside of | ||||
| Narwhals' scope - for example, when dealing with pandas' Period Dtype. | ||||
| 
     | 
||||
| Examples: | ||||
| >>> import narwhals as nw | ||||
| >>> import dask.dataframe as dd | ||||
| >>> lf_native = dd.from_dict({"a": [1, 2]}, npartitions=1) | ||||
| >>> nw.from_native(lf_native).implementation | ||||
| <Implementation.DASK: 'dask'> | ||||
| """ | ||||
| return self._compliant_frame._implementation | ||||
| 
     | 
||||
| def __getitem__(self, item: str | slice) -> NoReturn: | ||||
| msg = "Slicing is not supported on LazyFrame" | ||||
| raise TypeError(msg) | ||||
| 
          
            
          
           | 
    ||||
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| 
          
            
          
           | 
    @@ -11,7 +11,6 @@ | |
| import pytest | ||
| 
     | 
||
| import narwhals as nw | ||
| from narwhals._utils import is_eager_allowed | ||
| from narwhals.exceptions import ComputeError, InvalidOperationError | ||
| from tests.conftest import ( | ||
| dask_lazy_p1_constructor, | ||
| 
          
            
          
           | 
    @@ -114,11 +113,8 @@ def test_is_close_series_with_series( | |
| ) -> None: | ||
| df = nw.from_native(constructor_eager(data), eager_only=True) | ||
| x, y = df["x"], df["y"] | ||
| backend = df.implementation | ||
| assert is_eager_allowed(backend) | ||
| 
         There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π₯³  | 
||
| 
     | 
||
| nulls = nw.new_series( | ||
| name="nulls", values=[None] * len(x), dtype=nw.Float64(), backend=backend | ||
| "nulls", [None] * len(x), nw.Float64(), backend=df.implementation | ||
| ) | ||
| x = x.zip_with(x != NAN_PLACEHOLDER, x**0.5).zip_with(x != NULL_PLACEHOLDER, nulls) | ||
| y = y.zip_with(y != NAN_PLACEHOLDER, y**0.5).zip_with(y != NULL_PLACEHOLDER, nulls) | ||
| 
        
          
        
         | 
    @@ -141,11 +137,8 @@ def test_is_close_series_with_scalar( | |
| ) -> None: | ||
| df = nw.from_native(constructor_eager(data), eager_only=True) | ||
| y = df["y"] | ||
| backend = df.implementation | ||
| assert is_eager_allowed(backend) | ||
| 
     | 
||
| nulls = nw.new_series( | ||
| name="nulls", values=[None] * len(y), dtype=nw.Float64(), backend=backend | ||
| "nulls", [None] * len(y), nw.Float64(), backend=df.implementation | ||
| ) | ||
| y = y.zip_with(y != NAN_PLACEHOLDER, y**0.5).zip_with(y != NULL_PLACEHOLDER, nulls) | ||
| result = y.is_close(other, abs_tol=abs_tol, rel_tol=rel_tol, nans_equal=nans_equal) | ||
| 
          
            
          
           | 
    ||
Uh oh!
There was an error while loading. Please reload this page.
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.
Follow-up PR
Loooooooooong overdue at this stage, but I'm gonna move most of this stuff and some others into a new
narwhals._nativemodule which has:Protocols and aliases like:
narwhals/narwhals/_namespace.py
Lines 119 to 128 in 5d2f54f
Their corresponding new and re-aliased guards like:
narwhals/narwhals/_namespace.py
Lines 379 to 384 in 5d2f54f
And the
typing.Native*protocols as well:narwhals/narwhals/typing.py
Lines 24 to 39 in 5d2f54f
Beyond just organizing things, it'll mean we can deduplicate the definitions that appear in 3x typing modules π
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.
The type used for
sessionin (#3032 (comment)) would also make sense to be defined in this new module