Skip to content

chore: Spec CompliantSeries#2236

Merged
MarcoGorelli merged 24 commits intomainfrom
compliant-series-spec
Mar 20, 2025
Merged

chore: Spec CompliantSeries#2236
MarcoGorelli merged 24 commits intomainfrom
compliant-series-spec

Conversation

@dangotbanned
Copy link
Member

@dangotbanned dangotbanned commented Mar 17, 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

Look at all the *now* unused ignores!!!!
@dangotbanned dangotbanned changed the title chore(DRAFT): Spec CompliantSeries chore: Spec CompliantSeries Mar 18, 2025
@dangotbanned dangotbanned marked this pull request as ready for review March 18, 2025 20:28
Comment on lines +261 to +270
@property
def str(self) -> Any: ...
@property
def dt(self) -> Any: ...
@property
def cat(self) -> Any: ...
@property
def list(self) -> Any: ...
@property
def struct(self) -> Any: ...
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 wanna do these properly in a follow-up

Will be similar to EagerExprNamespace

class EagerExprNamespace(_ExprNamespace[EagerExprT], Generic[EagerExprT]):
def __init__(self, expr: EagerExprT, /) -> None:
self._compliant_expr = expr

raise AssertionError(msg)

def to_polars(self: Self) -> pl.DataFrame:
def to_polars(self: Self) -> pl.Series:
Copy link
Member Author

Choose a reason for hiding this comment

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

Discovering this one really was a surprise 😄

@MarcoGorelli
Copy link
Member

thanks, can you fix the conflicts please?

@dangotbanned
Copy link
Member Author

thanks, can you fix the conflicts please?

Sure thing @MarcoGorelli, won't be long

def quantile(
self,
quantile: float,
interpolation: Literal["nearest", "higher", "lower", "midpoint", "linear"],
Copy link
Member Author

@dangotbanned dangotbanned Mar 19, 2025

Choose a reason for hiding this comment

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

We should probably look into defining aliases for all of these Literal[...](s).

Or at least repeating the most common ones from polars

https://github.com/pola-rs/polars/blob/4cadeb91542de22698a038f7cbc4ef13ee6301dc/py-polars/polars/_typing.py

Copy link
Collaborator

@EdAbati EdAbati left a comment

Choose a reason for hiding this comment

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

This PR looks amazing and very satisfying to me 🙌

I'd like someone else to have a look and approve because I may have missed some communication/decisions of the past couple of weeks.

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 8aec8fa into main Mar 20, 2025
27 of 28 checks passed
@MarcoGorelli MarcoGorelli deleted the compliant-series-spec branch March 20, 2025 09:52
@dangotbanned
Copy link
Member Author

dangotbanned commented Mar 20, 2025

Thanks @EdAbati, @MarcoGorelli

This PR looks amazing and very satisfying to me 🙌

I'd like someone else to have a look and approve because I may have missed some communication/decisions of the past couple of weeks.

That is really useful feedback @EdAbati and echoes a similar comment (#2232 (comment)) from @FBruzzesi

I should probably organize a lot of the breadcrumbs (#2230) has put down into more of a narrative.
Some of the earlier PRs listed show off a lot of the benefits you might not see through the diff alone.

Big apologies for the visibility gap on this!

@FBruzzesi
Copy link
Member

I should probably organize a lot of the breadcrumbs (#2230) has put down into more of a narrative. Some of the earlier PRs listed show off a lot of the benefits you might not see through the diff alone.

@dangotbanned can you prepare a workshop/masterclass/livestream to get us to speed 😂? (Joking, but I would actually love that)

Big apologies for the visibility gap on this!

No need to apologize. Personally, I just had way less spare time in the last 3-4 weeks and chose to focus on some other features 🙈

@EdAbati
Copy link
Collaborator

EdAbati commented Mar 20, 2025

Same here, no need to apologise. It's my fault. I'm also only contributing in my spare time and this varies week by week 😅😅

I actually like a lot your PR comments and descriptions! They make navigating through past PRs and issues easier 🙂

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.

4 participants