Skip to content

MAINT: port numpy#13188 for np_datetime simplification #26516

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
Merged
merged 4 commits into from
May 26, 2019
Merged

MAINT: port numpy#13188 for np_datetime simplification #26516

merged 4 commits into from
May 26, 2019

Conversation

xcz011
Copy link
Contributor

@xcz011 xcz011 commented May 24, 2019

Bring numpy changes about emulating the behavior of python's divmod to
pandas.

xcz011 added 2 commits May 24, 2019 16:43
@xcz011
MAINT: port numpy#13188 for np_datetime simplificaion
6aadedd 
Bring numpy changes about emulating the behavior of python's divmod to
pandas.
@xcz011
cpplint fix
5804bad
@codecov
Copy link

codecov bot commented May 24, 2019

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #26516      +/-   ##
==========================================
- Coverage   91.76%   91.75%   -0.01%     
==========================================
  Files         174      174              
  Lines       50679    50679              
==========================================
- Hits        46504    46499       -5     
- Misses       4175     4180       +5
Flag Coverage Δ
#multiple 90.26% <ø> (ø) ⬆️
#single 41.7% <ø> (-0.11%) ⬇️
Impacted Files Coverage Δ
pandas/io/gbq.py 78.94% <0%> (-10.53%) ⬇️
pandas/core/frame.py 97% <0%> (-0.12%) ⬇️
pandas/util/testing.py 90.6% <0%> (-0.11%) ⬇️

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 b730ab3...5804bad. Read the comment docs.

@codecov
Copy link

codecov bot commented May 24, 2019 •
edited
Loading

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #26516      +/-   ##
==========================================
- Coverage   91.76%   91.74%   -0.02%     
==========================================
  Files         174      174              
  Lines       50679    50665      -14     
==========================================
- Hits        46504    46485      -19     
- Misses       4175     4180       +5
Flag Coverage Δ
#multiple 90.26% <ø> (-0.01%) ⬇️
#single 41.71% <ø> (-0.1%) ⬇️
Impacted Files Coverage Δ
pandas/io/gbq.py 78.94% <0%> (-10.53%) ⬇️
pandas/io/excel/_base.py 91.85% <0%> (-0.29%) ⬇️
pandas/core/frame.py 97% <0%> (-0.12%) ⬇️
pandas/util/testing.py 90.6% <0%> (-0.11%) ⬇️
pandas/core/api.py 100% <0%> (ø) ⬆️

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 b730ab3...e90d2e4. Read the comment docs.

@gfyoung gfyoung added Dependencies Required and optional dependencies Internals Related to non-user accessible pandas implementation labels May 25, 2019
gfyoung
gfyoung approved these changes May 25, 2019
View reviewed changes
Copy link
Member

@gfyoung gfyoung left a comment

Choose a reason for hiding this comment

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

Nice!

@jbrockmendel : If you want to have a look before merging

jbrockmendel
jbrockmendel reviewed May 25, 2019
View reviewed changes
pandas/_libs/tslibs/src/datetime/np_datetime.c
*
* Note that GCC is smart enough at -O2 to eliminate the `if(*d < 0)` branch
* for subsequent calls to this command - it is able to deduce that `*d >= 0`.
*/
Copy link
Member

Choose a reason for hiding this comment

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

Can you add to this comment pointing back to the numpy PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

just added it. Thanks! 😄

@jbrockmendel
Copy link
Member

Generally looks good. Were there any parts that weren't copy/paste?

@xcz011
Copy link
Contributor Author

xcz011 commented May 25, 2019

Generally looks good. Were there any parts that weren't copy/paste?

All changes are copied from numpy PR. I changed a few line style since those lines over 80 characters.

@jreback jreback added this to the 0.25.0 milestone May 26, 2019
@jreback
Copy link
Contributor

jreback commented May 26, 2019

lgtm. @jbrockmendel merge when ready.

@jbrockmendel jbrockmendel merged commit 0a516c1 into pandas-dev:master May 26, 2019
@jbrockmendel
Copy link
Member

Thanks @xcz011

another-green pushed a commit to another-green/pandas that referenced this pull request May 29, 2019
@xcz011 @another-green
MAINT: port numpy#13188 for np_datetime simplification (pandas-dev#26516
48a4b8c 
)

* MAINT: port numpy#13188 for np_datetime simplificaion

Bring numpy changes about emulating the behavior of python's divmod to
pandas.

* cpplint fix

* Add reference numpy change into comment

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

@jbrockmendel jbrockmendel jbrockmendel left review comments

@gfyoung gfyoung gfyoung approved these changes

Assignees
No one assigned
Labels
Dependencies Required and optional dependencies Internals Related to non-user accessible pandas implementation
Projects
None yet
Milestone
0.25.0
Development

Successfully merging this pull request may close these issues.

MAINT: port numpy#13188 for np_datetime simplification
4 participants
@xcz011 @jbrockmendel @jreback @gfyoung