Skip to content

STY: Spaces over concat strings - batch 1 #30707

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

Merged

Conversation

ShaharNaveh
Copy link
Member

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

@simonjayhawkins
Copy link
Member

I personally prefer the spaces at the start of the strings. I find it easier to spot missing spaces. not sure what others think.

@simonjayhawkins simonjayhawkins added the Code Style Code style, linting, code_checks label Jan 5, 2020
@WillAyd
Copy link
Member

WillAyd commented Jan 6, 2020

No strong preference but I think OK. @MomIsBestFriend can you merge master?

Copy link
Member

@datapythonista datapythonista left a comment

Choose a reason for hiding this comment

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

I'm more used to spaces at the end, but whatever it is, I think it's good to have a convention.

This comes from this new guide: https://dev.pandas.io/docs/development/code_style.html#id6 I think the standard started from a discussion in a review.

This PR lgtm, happy to get it merged. But also ok on reopening the discussion if the discussion when merging the style guide wasn't visible enough.

@WillAyd WillAyd added this to the 1.0 milestone Jan 6, 2020
@WillAyd WillAyd merged commit 04c3d51 into pandas-dev:master Jan 6, 2020
@WillAyd
Copy link
Member

WillAyd commented Jan 6, 2020

Thanks @MomIsBestFriend

@ShaharNaveh ShaharNaveh deleted the STY-spaces-over-concat-strings-1 branch January 6, 2020 23:42
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