-
Notifications
You must be signed in to change notification settings - Fork 166
feat: Add support for {Expr,Series}.str.to_titlecase
#3116
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
Conversation
Possibly helpful for #3116 (comment) Includes version handling via [`module.__getattr__`](https://docs.python.org/3/reference/datamodel.html#module.__getattr__) ### Note I'm not gonna push back if we explicitly **don't** want this one encouraged 😂: - #2370 - #2729
{Expr,Series}.str.to_titlecase
for non sql-like backends{Expr,Series}.str.to_titlecase
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.
pretty neat, thanks!
- Towards #3116 (comment) - Typing is easy to fix
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 @FBruzzesi - a few notes/suggestions from me - but looking good 🥳
def to_titlecase(self) -> SparkLikeExpr: | ||
impl = self.compliant._implementation | ||
sqlframe_required_version = (3, 43, 1) | ||
if ( | ||
impl.is_sqlframe() | ||
and (version := impl._backend_version()) < sqlframe_required_version | ||
): # pragma: no cover | ||
required_str = requires._unparse_version(sqlframe_required_version) | ||
found_str = requires._unparse_version(version) | ||
msg = ( | ||
f"`str.to_titlecase` is only available in 'sqlframe>={required_str}', " | ||
f"found version {found_str!r}." | ||
) | ||
raise NotImplementedError(msg) |
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 guess this would be a case that #3059 would be nice for?
This'll do for now though 😄
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 @FBruzzesi, my suggestions aren't blocking - looking great
Co-authored-by: Dan Redding <[email protected]>
What type of PR is this? (check all applicable)
Related issues
str.to_titlecase
#3060Checklist
If you have comments or can explain your changes, please do so below