Skip to content

Conversation

@DorAmram
Copy link
Contributor

@pep8speaks
Copy link

pep8speaks commented Dec 22, 2019

Hello @DorAmram! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2019-12-22 18:03:07 UTC

Copy link
Contributor

@topper-123 topper-123 left a comment

Choose a reason for hiding this comment

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

Looks good, a single issue.

root=root, dirname=dirname, parentdir_prefix=parentdir_prefix
)
f"""guessing rootdir is '{root}', but '{dirname}'
doesn't start with prefix '{parentdir_prefix}'"""
Copy link
Contributor

Choose a reason for hiding this comment

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

don't use triple-quoted strings, it's not the same as two single-quoted strings.

@topper-123 topper-123 added this to the 1.0 milestone Dec 22, 2019
@topper-123 topper-123 added Code Style Code style, linting, code_checks and removed Clean labels Dec 23, 2019
@alimcmaster1
Copy link
Member

Hmm I think this file is autogenerated as per @jbrockmendel comments here #29518 (comment)

@jreback
Copy link
Contributor

jreback commented Dec 23, 2019

yeah we can leave this one alone, do we have a check for .format usage in checks? (prob not yet)

@alimcmaster1
Copy link
Member

yeah we can leave this one alone, do we have a check for .format usage in checks? (prob not yet)

Not yet there are still 200+ files to tackle.
grep -l -R -e '%s' -e '%d' -e '\.format(' --include=*.{py,pyx} pandas/

If we want to add a check now we can do - and users remove the file from exclude list once done?
(Would help confirm all instances of .format have been removed in the PR for a file)

@jbrockmendel
Copy link
Member

grepping for .format( will be complicated by the fact that we have 6 places where we def format\(. But we can worry about that later. This PR is non-actionable, closing.

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.

6 participants