Skip to content
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

CI: Checking in f-strings we use {repr(...)} instead of {...!r} #30099

Merged
merged 1 commit into from
Dec 25, 2019

Conversation

ShaharNaveh
Copy link
Member

@ShaharNaveh ShaharNaveh commented Dec 5, 2019

@simonjayhawkins simonjayhawkins added the Code Style Code style, linting, code_checks label Dec 6, 2019
@WillAyd
Copy link
Member

WillAyd commented Dec 17, 2019

@MomIsBestFriend I think all precursors are done (?) can you merge master?

@ShaharNaveh
Copy link
Member Author

@WillAyd sorry for the delay, was on vacation.

Do you have an idea on how to exclude patterns like %%r?

@jbrockmendel
Copy link
Member

Do you have an idea on how to exclude patterns like %%r?

You want to grep for "%r" but exclude "%%r" right?

With regular grep you could do grep -R --include=*.{py,pyx} '%r' pandas | grep -v '%%r'. I'm not sure what it will take to make this work with the invgrep in the code checks

@ShaharNaveh
Copy link
Member Author

ShaharNaveh commented Dec 21, 2019

Do you have an idea on how to exclude patterns like %%r?

You want to grep for "%r" but exclude "%%r" right?

With regular grep you could do grep -R --include=*.{py,pyx} '%r' pandas | grep -v '%%r'. I'm not sure what it will take to make this work with the invgrep in the code checks

That's an interesting way of doing it, the problem is that the return status is messing my head. because the invgrep is giving the opposite return status of grep. I just changed the matching pattern to make my life easier.

Hope my way of thinking is good.

@jbrockmendel
Copy link
Member

@MomIsBestFriend pls rebase. did you figure out the grep issue?

@ShaharNaveh
Copy link
Member Author

did you figure out the grep issue?

Yes, I am greping for <Space>%r because it's more common, all the rest are edge cases, so it's not really worth writing test cases for them.

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.

Thanks for adding those @MomIsBestFriend

The second rule looks ok, but I'm unsure about the first one.

ci/code_checks.sh Outdated Show resolved Hide resolved
@ShaharNaveh ShaharNaveh force-pushed the CI-TestCase-#29886 branch 2 times, most recently from 093e93c to f1dfa20 Compare December 25, 2019 20:26
@ShaharNaveh
Copy link
Member Author

Sorry for the short mess, messed something with the HEAD and the upstream/maser pointers.

It's all good now, correct?

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.

lgtm, thanks @MomIsBestFriend

@datapythonista datapythonista changed the title CI: Unwanted patterns checks CI: Checking in f-strings we use {repr(...)} instead of {...!r} Dec 25, 2019
@jreback jreback added this to the 1.0 milestone Dec 25, 2019
@jreback jreback merged commit b3335d7 into pandas-dev:master Dec 25, 2019
@jreback
Copy link
Contributor

jreback commented Dec 25, 2019

thanks @MomIsBestFriend

@ShaharNaveh ShaharNaveh deleted the CI-TestCase-#29886 branch December 25, 2019 22:23
AlexKirko pushed a commit to AlexKirko/pandas that referenced this pull request Dec 29, 2019
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