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.

Sign up for GitHub

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

Jump to bottom

CLN: post-post numpy bump #24365

Merged
merged 2 commits into from
Dec 22, 2018
Merged

CLN: post-post numpy bump #24365

merged 2 commits into from
Dec 22, 2018

Conversation

h-vetinari
Copy link
Contributor

@h-vetinari h-vetinari commented Dec 19, 2018 •
edited
Loading

Even after #23062 and #23796, I still keep finding occurrences of old numpy references in the code. Gotta catch 'em all...

@codecov
Copy link

codecov bot commented Dec 19, 2018

Codecov Report

Merging #24365 into master will decrease coverage by 49.3%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           master   #24365       +/-   ##
===========================================
- Coverage   92.29%   42.98%   -49.31%     
===========================================
  Files         162      162               
  Lines       51832    51832               
===========================================
- Hits        47839    22281    -25558     
- Misses       3993    29551    +25558
Flag Coverage Δ
#multiple ?
#single 42.98% <ø> (ø) ⬆️
Impacted Files Coverage Δ
pandas/io/formats/latex.py 0% <0%> (-100%) ⬇️
pandas/core/categorical.py 0% <0%> (-100%) ⬇️
pandas/io/sas/sas_constants.py 0% <0%> (-100%) ⬇️
pandas/tseries/plotting.py 0% <0%> (-100%) ⬇️
pandas/tseries/converter.py 0% <0%> (-100%) ⬇️
pandas/io/formats/html.py 0% <0%> (-98.65%) ⬇️
pandas/core/groupby/categorical.py 0% <0%> (-95.46%) ⬇️
pandas/io/sas/sas7bdat.py 0% <0%> (-91.17%) ⬇️
pandas/io/sas/sas_xport.py 0% <0%> (-90.15%) ⬇️
pandas/core/tools/numeric.py 10.44% <0%> (-89.56%) ⬇️
... and 120 more

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 f6cf7d9...5b60f51. Read the comment docs.

@codecov
Copy link

codecov bot commented Dec 19, 2018 •
edited
Loading

Codecov Report

Merging #24365 into master will decrease coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #24365      +/-   ##
==========================================
- Coverage   92.29%   92.29%   -0.01%     
==========================================
  Files         162      162              
  Lines       51832    51832              
==========================================
- Hits        47839    47838       -1     
- Misses       3993     3994       +1
Flag Coverage Δ
#multiple 90.7% <ø> (ø) ⬆️
#single 42.98% <ø> (ø) ⬆️
Impacted Files Coverage Δ
pandas/util/testing.py 87.57% <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 f6cf7d9...5b60f51. Read the comment docs.

@h-vetinari h-vetinari changed the title CLN: post-post numpy bumpy CLN: post-post numpy bump Dec 20, 2018
TomAugspurger
TomAugspurger reviewed Dec 21, 2018
View reviewed changes
pandas/_libs/tslibs/timedeltas.pyx
@@ -161,9 +161,6 @@ cpdef convert_to_timedelta64(object ts, object unit):
- None/NaT

Return an ns based int64

# kludgy here until we have a timedelta scalar
# handle the numpy < 1.7 case
Copy link
Contributor

@TomAugspurger TomAugspurger Dec 21, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Do you know what section of code this was referring to (I don't immediately see one)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@TomAugspurger
I don't see something particularly kludge-y either. Also, there is a timedelta scalar nowadays. It just seems like a severely outdated comment.

TomAugspurger
TomAugspurger approved these changes Dec 21, 2018
View reviewed changes
Copy link
Contributor

@TomAugspurger TomAugspurger left a comment

Choose a reason for hiding this comment

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

Just one tiny question, though this should be good to go.

@jreback jreback added Clean Dependencies Required and optional dependencies labels Dec 21, 2018
@jreback jreback added this to the 0.24.0 milestone Dec 21, 2018
@jreback
Copy link
Contributor

jreback commented Dec 21, 2018

lgtm. ex @TomAugspurger question

h-vetinari
h-vetinari commented Dec 21, 2018
View reviewed changes
Copy link
Contributor Author

@h-vetinari h-vetinari left a comment

Choose a reason for hiding this comment

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

@TomAugspurger @jreback
Responded in comment

pandas/_libs/tslibs/timedeltas.pyx
@@ -161,9 +161,6 @@ cpdef convert_to_timedelta64(object ts, object unit):
- None/NaT

Return an ns based int64

# kludgy here until we have a timedelta scalar
# handle the numpy < 1.7 case
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@TomAugspurger
I don't see something particularly kludge-y either. Also, there is a timedelta scalar nowadays. It just seems like a severely outdated comment.

@TomAugspurger
Copy link
Contributor

Agreed, thanks.

@TomAugspurger TomAugspurger merged commit 3e0358d into pandas-dev:master Dec 22, 2018
@h-vetinari h-vetinari deleted the post_post_numpy_bump branch December 22, 2018 23:47
Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019
@h-vetinari @Pingviinituutti
CLN: post-post numpy bump (pandas-dev#24365)
b85f044 
* CLN: post-post numpy bumpy

* Lint
Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019
@h-vetinari @Pingviinituutti
CLN: post-post numpy bump (pandas-dev#24365)
49f339b 
* CLN: post-post numpy bumpy

* Lint
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mroeschke mroeschke mroeschke approved these changes

@TomAugspurger TomAugspurger TomAugspurger approved these changes

Assignees
No one assigned
Labels
Clean Dependencies Required and optional dependencies
Projects
None yet
Milestone
0.24.0
Development

Successfully merging this pull request may close these issues.
4 participants
@h-vetinari @jreback @TomAugspurger @mroeschke