-
Notifications
You must be signed in to change notification settings - Fork 18
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
Check for .groupby aggregation patterns #24
Comments
Both of the anti-patterns have
(AST chains can get crazy) Do you see any cases where this pattern wouldn't work (i.e., other |
Turns out the AST syntax is actually more complex when the Unless there are cases where that syntax might be appropriate. |
This is my fear. I am having second thoughts about this because I am going to suggest that we should hold off on implementing this until we have a better survey of |
Okay. I have a preliminary commit that I will probably push and create a
pull request, if only as a reference for discussion of the implementation.
Although if following strictly the recommendation in the Minimally
Sufficient Pandas article, we can probably capture the specific pattern
where a slice is applied to the groupby method. And there if someone wants
to use that syntax anyway, they always have the `# noqa` flag that they can
use to ignore. (I don't really know too much about this Flake8 mechanism,
but it appears that you can either specify for individual lines, or turn
off the check via a command line option.)
…On Fri, Mar 8, 2019 at 11:48 AM Jacob Deppen ***@***.***> wrote:
Unless there are cases where that syntax might be appropriate.
This is my fear. I am having second thoughts about this because groupby
is EXTREMELY common in pandas code and I am afraid there are probably
lots of cases I am not considering.
I am going to suggest that we should hold off on implementing this until
we have a better survey of groupby in the wild. I will open an issue
where we can collect examples.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#24 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AB2sGt9GNcmwXTqqttZfItsz-k_A6SIcks5vUr50gaJpZM4bSzl2>
.
|
Not even sure if this can be implemented, but maybe with a clever regular expression, it could. See the flashcard for some more details.
The text was updated successfully, but these errors were encountered: