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

if minp > win_n:
raise ValueError(f"min_periods (minp) must be <= "
f"window (win)")
raise ValueError(f"min_periods {minp} must be <= window {win_n}")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The f did nothing before. should I revert and just delete the f?
or leave it as it is?

Copy link
Member

Choose a reason for hiding this comment

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

I think leave the message unchanged unless obvious improvement and then should add a brief note in whatsnew that message has been changed.

Copy link
Contributor Author

@ShaharNaveh ShaharNaveh Jan 12, 2020

Choose a reason for hiding this comment

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

I think I'll just delete this f because it didn't do anything anyway.

Copy link
Member

@simonjayhawkins simonjayhawkins left a comment

Choose a reason for hiding this comment

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

Thanks @MomIsBestFriend ltgm however probably best in general to leave cython files untouched and wait for black to support formatting cython.

if minp > win_n:
raise ValueError(f"min_periods (minp) must be <= "
f"window (win)")
raise ValueError(f"min_periods {minp} must be <= window {win_n}")
Copy link
Member

Choose a reason for hiding this comment

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

I think leave the message unchanged unless obvious improvement and then should add a brief note in whatsnew that message has been changed.

Co-Authored-By: Simon Hawkins <simonjayhawkins@gmail.com>
@ShaharNaveh
Copy link
Contributor Author

best in general to leave cython files untouched and wait for black to support formatting cython.

@simonjayhawkins should I close this PR then?

@simonjayhawkins
Copy link
Member

best in general to leave cython files untouched and wait for black to support formatting cython.

@simonjayhawkins should I close this PR then?

I think OK to merge this once the changed message #30942 (comment) is reverted

@simonjayhawkins simonjayhawkins added the Code Style Code style, linting, code_checks label Jan 12, 2020
@simonjayhawkins simonjayhawkins added this to the 1.1 milestone Jan 12, 2020
Copy link
Member

@WillAyd WillAyd left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Member

@simonjayhawkins simonjayhawkins left a comment

Choose a reason for hiding this comment

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

Thanks @MomIsBestFriend lgtm. ping on green.

@ShaharNaveh
Copy link
Contributor Author

ping @simonjayhawkins

@simonjayhawkins simonjayhawkins merged commit 62d16ab into pandas-dev:master Jan 13, 2020
@simonjayhawkins
Copy link
Member

Thanks @MomIsBestFriend

@ShaharNaveh ShaharNaveh deleted the STY-shorter-strings branch January 13, 2020 14:52
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.

3 participants