-
Notifications
You must be signed in to change notification settings - Fork 62
Implement initial version of rewrite for df.getitem as attr #601
Implement initial version of rewrite for df.getitem as attr #601
Conversation
88facdf to
b6e1dad
Compare
| if not isinstance(obj, DataFrameType): | ||
| continue | ||
| if expr.attr in obj.columns: | ||
| getattrs.add(expr) |
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.
So we won't rewrite if we didn't find valid column name, right?
This means we won't raise AttributeError as pandas does if we use an invalid attribute name. We should probably state that in limitations, or think if we can look for all_supported_dataframe_methods in this rewrite and if we actually call a method skip rewriting, which aligns to pandas behavior:
>>>df
A B values
0 2 -8 -8
1 1 2 2
2 1 3 3
3 1 1 1
4 2 5 5
5 2 6 6
6 1 7 7
>>>df.values
array([[ 2, -8, -8],
[ 1, 2, 2],
[ 1, 3, 3],
[ 1, 1, 1],
[ 2, 5, 5],
[ 2, 6, 6],
[ 1, 7, 7]], dtype=int64)
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.
Yes, we need to think about it, but in next PR
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.
Right now, the main issue us the case when column is named like '_data', '_columns' or '_index'. This will break all uses of dataframe
kozlov-alexey
left a comment
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.
@densmirn Cool feature! 🥇 Have you felt yourself a magician during writing this?
At the least I liked working on the 😄 |
No description provided.