Skip to content

Merge with main and demo fixes#36

Merged
tswast merged 8 commits into
mainfrom
ab_indexing
May 28, 2026
Merged

Merge with main and demo fixes#36
tswast merged 8 commits into
mainfrom
ab_indexing

Conversation

@andyorange
Copy link
Copy Markdown
Collaborator

@andyorange andyorange commented May 27, 2026

Hi Tim,
I merged the current main branch into an_indexing and fixed the demos by re-adding the filter_by method to the DataFrameHandler. Indexing and nesting not work again!
Would love to see the branch merged into main. Also, Vodafone would love to see me mentioned as a collaborator for the project...
The remaining issues are from the linter and refer to files you created, I do not want to interfere here and leave them changes to you

P.S.: I created the demos with Chat GPT. As that is also google, I hope there are no rights conflicts.

@andyorange andyorange requested a review from tswast May 27, 2026 05:25
Copy link
Copy Markdown
Owner

@tswast tswast left a comment

Choose a reason for hiding this comment

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

Thanks!

Comment thread demos/demo_indexing_with_nested.py Outdated

# Set index on timestamp
print("\n📍 Setting index on 'timestamp' (ascending)...")
df_indexed = df.set_index('timestamp', ascending=True)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

The ascending=True is a departure from pandas.DataFrame.set_index, but I think this is okay, because:

  1. Most of the time folks would expect some kind of ordering to be available when they set an index, so that iloc can be used.
  2. The ascending=True naming is consistent with how ordering is defined in pandas.

Comment thread docs/indexing_guide.md
df_ordered = DataFrame(ordered)

# Now use indexing
df_ordered._index = Index('priority', ascending=False)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I'm a bit confused what the intention of this line is here? I guess it's so that we don't override the sorting introduced in the ibis expression?

Comment thread leanframe/core/frame.py

def __init__(self, data: ibis_types.Table):
self._data = data
self._index: Index | None = None # Explicit ordering specification
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I'm curious if we'd want to separate the ordering from the index definition for a bit better compatibility with pandas, like how bigframes does it? I suspect that adds an unnecessary level of compexity?

Comment thread leanframe/core/frame.py
path: info["extracted_name"] for path, info in self.nested_fields.items()
}

def filter_by(self, **kwargs) -> "DataFrameHandler":
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This also is a departure from pandas, but I do quite like it. Happy to keep it. In regular pandas, basically every filtering method I can think of requires an implicit join on the index column.

@tswast tswast merged commit d82a4c1 into main May 28, 2026
5 checks passed
@andyorange
Copy link
Copy Markdown
Collaborator Author

andyorange commented May 29, 2026 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants