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

CLN: replace %s syntax with .format in tslibs/timedeltas #18405

Merged
merged 1 commit into from
Nov 22, 2017

Conversation

WillAyd
Copy link
Member

@WillAyd WillAyd commented Nov 21, 2017

@WillAyd
Copy link
Member Author

WillAyd commented Nov 21, 2017

FWIW I didn't see what the point of the 'even_day' argument was to _repr_base in timedeltas.pyx. From both the test cases provided and the control flow, unless I'm missing something that argument is no different from passing nothing or None explicitly.

Left alone for now but if you want me to remove it won't be a problem

@jreback jreback added the Code Style Code style, linting, code_checks label Nov 21, 2017
@jreback
Copy link
Contributor

jreback commented Nov 21, 2017

lgtm. IIRC even_day was copied from Timestamp originally (when I wrote this repr). if its not used, you can remove.

@WillAyd
Copy link
Member Author

WillAyd commented Nov 21, 2017

I’ll go ahead and update the docstring to remove. Did you want to keep the test cases for backwards compatibility or would you rather I delete and just stick with the cases for None?

@jreback
Copy link
Contributor

jreback commented Nov 21, 2017

I’ll go ahead and update the docstring to remove. Did you want to keep the test cases for backwards compatibility or would you rather I delete and just stick with the cases for None?

ok to delete; these are only tests, yes?

@WillAyd
Copy link
Member Author

WillAyd commented Nov 21, 2017

It looks like the tests and one piece of code in pandas/io/formats/format.py at line 2277 to change

format = ‘even_day’

To

format = None

It looks like there’s already a test case called test_timedelta in the tests/io/formats/test_format.py module to regression test that.

Sorry for any formatting issues - sent from my phone.

@codecov
Copy link

codecov bot commented Nov 21, 2017

Codecov Report

Merging #18405 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #18405      +/-   ##
==========================================
- Coverage   91.36%   91.34%   -0.02%     
==========================================
  Files         164      164              
  Lines       49733    49733              
==========================================
- Hits        45439    45430       -9     
- Misses       4294     4303       +9
Flag Coverage Δ
#multiple 89.14% <100%> (ø) ⬆️
#single 39.62% <0%> (-0.07%) ⬇️
Impacted Files Coverage Δ
pandas/io/formats/format.py 96.01% <100%> (ø) ⬆️
pandas/io/gbq.py 25% <0%> (-58.34%) ⬇️
pandas/core/frame.py 97.8% <0%> (-0.1%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c4a2cd3...03e4d41. Read the comment docs.

@jreback jreback added this to the 0.22.0 milestone Nov 22, 2017
@jreback jreback merged commit 2dbf2a6 into pandas-dev:master Nov 22, 2017
@jreback
Copy link
Contributor

jreback commented Nov 22, 2017

thanks!

@WillAyd WillAyd deleted the td-str-format branch November 22, 2017 00:28
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.

None yet

2 participants