refactor: remove ultra magical __getitem__ support

[cpcloud]

I'd like to propose the removal of filtering and projection support from __getitem__ and retain only the ability to select single columns from a table expression. __getitem__ functionality from column expressions would be unchanged

The rationale is that the behavior of these methods is hard to predict without semantic understanding of the expressions involved in the call.

Yyou cannot know whether the resulting expression is a projection or a filter without knowing details about the expressions involved.

Here's a simple example of this:

t = ... # table expression
t[t.a]  # is this a projection or filter?
t[[t.a]] # is this the same as ☝🏻?

In contrast, when you see a call to the select method and a call to filter method you know what the resulting expression is.

This would need to go through a deprecation cycle with a few releases before removing it.

[cpcloud]

Well, it looks like there's no way to know who voted and how (https://github.com/orgs/community/discussions/17367), so I'll just start by saying +1.

[gforsyth]

I'm a +1 on this

[jcrist]

Also +1.

[NickCrews]

also +1, great idea.

[NickCrews]

EDIT: I didn't read the original post well enough, turns out we are mostly on the same page.

I like the shorthand of __getitem__

• it fits in with the general python pattern of "getting something from a container"
• it is familiar for pandas users
• we can make it consistent, we just have to remove the .filter() functionality. If you want to filter, you have to call .filter().

I have never liked how t[x] gives you a ColumnValue, but t[x,y] gives a Table back even though you just added a single indexer. If we're being breaking I would vote to make this consistent as well:

(in all of these, x, y and z can either be strings or more complex expressions)

• t[x]: ColumnValue. NEVER filters. IDK I suppose this could be a footgun for people coming from pandas, they do this thinking it is a filter, but it isn't. In most cases they would discover their error immediately, but there are cases when the error could silently pass...
• t[[x]]: a Table with a single column
• t[[x, y, z]]: A Table with multiple columns
• t[x, y, z]: Error, "did you mean t[[x, y, z]]?"

[cpcloud]

The following things would remain in my proposal:

1. Single column access with a string t["a"]
2. Multiple column access with a list of strings t[["a", "b", "c"]]
3. Table row slicing t[start:end]

What is removed is:

1. Complex expression projection. I think this is inconsitent with the rest of the pydata ecosystem and Python generally. __getitem__ should be a about getting stuff out, not about computing new expressions. For example, is t[t.x + 1] equivalent to t.x + 1 or is it a single column table with a new expression? I 100% think we should disallow complex expressions in __getitem__.
2. Filtering (though I think removing complex projections would make this unambiguous, i.e., an t[expr] would always mean filter). I still think we should remove it to reduce API surface.
3. tuple keys e.g., t["a", "b"]

The primary piece of magic that I think is confusing is being able to construct new table expressions that are not simple subsets of columns with __getitem__.

[NickCrews]

Oops, sorry I didn't read your original post well enough. Sounds like we are pretty much in agreement!

OK I think you've sold me on disallowing the complex expression projection (t[t.x + 1]) feature. One pro of disallowing that you don't mention is that it would remove the footgun I mention, as t[_.my_col.notna()] would simply fail. It is easier to explain to people too.

[NickCrews]

Oh, one other related thought: to make it a bit more ergonomic, what if we changed the API from .filter(predicates: Iterable[BooleanValue]), to .filter(*predicates: BooleanValue | Iterable[BooleanValue], /)? This would allow t.filter(_.x > 0, _.y == 6) without needing to wrap the conditions in the extra iterable t.filter([_.x > 0, _.y == 6]), or needing to add parens with t.filter((_.x > 0) & (_.y == 6)) The / in the signature allows us to add keyword args in the future if we ever wanted to.

[cpcloud]

Just following up here after a while :)

This particular API change was made a while ago!

[cpcloud]

I'm going to convert this to an issue so we can track it under API stability.

[cpcloud]

Well, I guess that's not supported, wonderful!