Skip to content

refactor(typing): make _evaluate_output_names generic#2053

Merged
MarcoGorelli merged 3 commits intomainfrom
dev-pyright-eval-out-names
Feb 22, 2025
Merged

refactor(typing): make _evaluate_output_names generic#2053
MarcoGorelli merged 3 commits intomainfrom
dev-pyright-eval-out-names

Conversation

@dangotbanned
Copy link
Member

@dangotbanned dangotbanned commented Feb 20, 2025

What type of PR is this? (check all applicable)

  • 💾 Refactor
  • ✨ Feature
  • 🐛 Bug Fix
  • 🔧 Optimization
  • 📝 Documentation
  • ✅ Test
  • 🐳 Other

Related issues

Checklist

  • Code follows style guide (ruff)
  • Tests added
  • Documented the changes

If you have comments or can explain your changes, please do so below

Follow up to (#2035), with a change that solves the last 10 errors.

Important

The commit history wasn't clean
PR branched b26c94c (#2035)

As mentioned in (#2035 (comment)), there are some similar changes in reuse-sub-protocols

That branch will need some tweaks after this, but it will also benefit - as it currently types this as Any to avoid the same error:

_evaluate_output_names: Any
_alias_output_names: Any
_implementation: Implementation
_backend_version: tuple[int, ...]
_version: Version
_kwargs: dict[str, Any]
def __init__(
self: Self,
call: Callable[[CompliantDataFrameT_co], Sequence[ReuseSeriesT]],
*,
depth: int,
function_name: str,
evaluate_output_names: Callable[[CompliantDataFrameT_co], Sequence[str]],


In (#2064), I've been using the aliases (EvalSeries, EvalNames) which I think give a huge boost to readability:

class CompliantSeriesWithDType(CompliantSeries, Protocol):
@property
def dtype(self) -> DType: ...
SeriesT = TypeVar("SeriesT", bound="CompliantSeriesWithDType")
FrameT = TypeVar("FrameT", bound="CompliantDataFrame | CompliantLazyFrame")
SelectorOrExpr: TypeAlias = "CompliantSelector[FrameT, SeriesT] | CompliantExpr[SeriesT]"
EvalSeries: TypeAlias = Callable[[FrameT], Sequence[SeriesT]]
EvalNames: TypeAlias = Callable[[FrameT], Sequence[str]]

vs main:

class CompliantExpr(Protocol, Generic[CompliantSeriesT_co]):
_implementation: Implementation
_backend_version: tuple[int, ...]
_version: Version
_evaluate_output_names: Callable[
[CompliantDataFrame | CompliantLazyFrame], Sequence[str]
]
_alias_output_names: Callable[[Sequence[str]], Sequence[str]] | None
_depth: int
_function_name: str

Comment on lines -40 to -42
ArrowOrPandasLikeExpr = TypeVar(
"ArrowOrPandasLikeExpr", bound=Union[ArrowExpr, PandasLikeExpr]
)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was playing around with fixing the ignores - noticed this was unused

@dangotbanned dangotbanned marked this pull request as ready for review February 20, 2025 13:09
@dangotbanned dangotbanned marked this pull request as draft February 21, 2025 22:50
dangotbanned added a commit that referenced this pull request Feb 22, 2025
Typing correctly will require (#2053), so leaving it out for now
@dangotbanned dangotbanned force-pushed the dev-pyright-eval-out-names branch from 69b81e7 to 07c6e6e Compare February 22, 2025 15:44
@dangotbanned dangotbanned marked this pull request as ready for review February 22, 2025 16:15
Copy link
Member

@MarcoGorelli MarcoGorelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @dangotbanned !

@MarcoGorelli MarcoGorelli merged commit 38675ab into main Feb 22, 2025
30 of 31 checks passed
@MarcoGorelli MarcoGorelli deleted the dev-pyright-eval-out-names branch February 22, 2025 18:17
@dangotbanned
Copy link
Member Author

thanks @dangotbanned !

Thanks @MarcoGorelli!

I botched the git history in at least 3 different ways on this PR 😅
Very happy its off my hands now

dangotbanned added a commit that referenced this pull request Feb 23, 2025
MarcoGorelli pushed a commit that referenced this pull request Feb 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants