Skip to content

perf: Avoid redefining lambda in *Namespace.all#2102

Merged
dangotbanned merged 2 commits intomainfrom
perf-get-column-names
Feb 26, 2025
Merged

perf: Avoid redefining lambda in *Namespace.all#2102
dangotbanned merged 2 commits intomainfrom
perf-get-column-names

Conversation

@dangotbanned
Copy link
Member

@dangotbanned dangotbanned commented Feb 26, 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

Previously, a new function was created in each of these locations - but it didn't bind any arguments - so there was no benefit to using a lambda

@dangotbanned dangotbanned marked this pull request as ready for review February 26, 2025 13:41
@MarcoGorelli
Copy link
Member

thanks - could you clarify how this helps please? i don't understand the difference, and there aren't any arguments to bind in this function?

@dangotbanned
Copy link
Member Author

dangotbanned commented Feb 26, 2025

thanks - could you clarify how this helps please? i don't understand the difference,

Sure thing @MarcoGorelli

Right now, every time all() is called that line is equivalent to:

def _(df):
    return df.columns
...
columns = _(df)

By equivalent - I mean if you call all() 3 times - that would expand to this:

def _1(df):
    return df.columns
...
columns = _1(df)
def _2(df):
    return df.columns
...
columns = _2(df)
def _3(df):
    return df.columns
...
columns = _3(df)

This change skips defining function each time, so those 3 calls would all be:

columns = get_column_names(df)
columns = get_column_names(df)
columns = get_column_names(df)

The difference is we didn't need to create a new inline function each time.

and there aren't any arguments to bind in this function?

What I mean is the scope created by lambda df: df.columns doesn't bind an argument in the definition.
By comparison, this lambda binds self._implementation, self._backend_version, self._version from the outer scope.

That would be a use-case I'd understand for a lambda - since it utilizes self

def all(self: Self) -> PandasLikeExpr:
return PandasLikeExpr(
lambda df: [
PandasLikeSeries(
df._native_frame[column_name],
implementation=self._implementation,
backend_version=self._backend_version,
version=self._version,

Also some links:

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.

sure, thanks @dangotbanned

@dangotbanned dangotbanned merged commit b362e46 into main Feb 26, 2025
27 of 28 checks passed
@dangotbanned dangotbanned deleted the perf-get-column-names branch February 26, 2025 14:10
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