Skip to content

comparison_with_sql document: adding a calculated column #28162

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

Closed
DavidRosen opened this issue Aug 27, 2019 · 8 comments
Closed

comparison_with_sql document: adding a calculated column #28162

DavidRosen opened this issue Aug 27, 2019 · 8 comments
Labels
Milestone

Comments

@DavidRosen
Copy link
Contributor

DavidRosen commented Aug 27, 2019

https://pandas-docs.github.io/pandas-docs-travis/getting_started/comparison/comparison_with_sql.html

In section "SELECT", the important use case of adding a calculated column is omitted.

I suggest adding something like the following:

select *, tip/total_bill as tip_rate from tips

pd.concat([tips, pd.Series(tips.tip/tips.total_bill, name='tip_rate')], axis=1)

@TomAugspurger
Copy link
Contributor

I would write this as

tips.assign(
    tip_rate = tips['tip'] / tips['total_bill']
)

are you interested in adding this example @DavidRosen?

@DavidRosen
Copy link
Contributor Author

are you interested in adding this example @DavidRosen?

@TomAugspurger, I'm a novice and really don't know what's involved in adding to the docs.

@TomAugspurger
Copy link
Contributor

TomAugspurger commented Aug 27, 2019 via email

@DavidRosen
Copy link
Contributor Author

Come to think of it, why do we even need .assign() ? How about just:

tips['tip_rate'] = tips['tip'] / tips['total_bill']

BTW, this also has the advantage that the linter won't prohibit spaces around the "=" as it did when we used .assign() (because "=" was for a keyword argument).

@lucassa3
Copy link
Contributor

lucassa3 commented Aug 29, 2019

Hi @TomAugspurger, I just referenced this issue in a PR a while ago. I'm relatively new to contributing to open source projects, so I'd like you to review if this is what was proposed in this Issue.

Best,

Lucas

@TomAugspurger
Copy link
Contributor

Hi @lucassa3. I think David submitted a PR but didn’t reference the issue. So we won’t need your PR for this issue.

It looks like you made your PR against your master branch. In the future you should target pandas’ master branch. Let us know if you need help finding another issue.

@lucassa3
Copy link
Contributor

lucassa3 commented Sep 3, 2019

Yes, I accidentally forgot to change merge target when opening the PR. I'll check other issues around, but thanks for the feedback Tom!

@TomAugspurger
Copy link
Contributor

Closed by #28182.

@TomAugspurger TomAugspurger added this to the 1.0 milestone Sep 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants