Skip to content

Conversation

ShaharNaveh
Copy link
Contributor

  • closes #xxxx
  • tests added / passed
  • passes black pandas
  • passes git diff upstream/master -u -- "*.py" | flake8 --diff
  • whatsnew entry

num=num, size_q=size_qualifier, x=x
)
num /= 1024.0
num /= 1_024.0
Copy link
Member

Choose a reason for hiding this comment

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

i dont think this adds clarity. AFAIK we dont have a policy on this, but maybe something like 7+ digits?

Copy link
Member

Choose a reason for hiding this comment

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

I agree, I also think for relatively small numbers this does not improve readability IMO.

Also, I don't think we need to do this for random numbers in the tests. Yes to using underscores in meaningful numbers (such as in ccalendar.DAY_SECONDS * 1_000_000_000), but for random values in tests asserts, I don't see the added value.

@jreback jreback added the Code Style Code style, linting, code_checks label Dec 10, 2019
@jreback jreback added this to the 1.0 milestone Dec 10, 2019
@jreback jreback merged commit 71cdd50 into pandas-dev:master Dec 10, 2019
@jreback
Copy link
Contributor

jreback commented Dec 10, 2019

thanks @MomIsBestFriend

yeah agree with the others, this is for reading clarify, so want to use it mainly on long static constants.

proost pushed a commit to proost/pandas that referenced this pull request Dec 19, 2019
proost pushed a commit to proost/pandas that referenced this pull request Dec 19, 2019
@ShaharNaveh ShaharNaveh deleted the STY-underscores-batch-2 branch December 20, 2019 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code Style Code style, linting, code_checks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants