Skip to content

Conversation

@jbrockmendel
Copy link
Member

tslib.get_value_box is effectively identical to index.get_value_at (figuring this out requires looking at util.get_value_at). Since the only use of get_value_box is in core.index.base, just get rid of it and use libindex.get_value_at there instead.

Fixup type declarations in util.get_value_at.

@jreback
Copy link
Contributor

jreback commented Nov 19, 2017

pls asv indexing (shouldn't be an issue, but just check).

@jreback jreback added Clean Indexing Related to indexing on series/frames, not to indexes themselves Datetime Datetime data dtype labels Nov 19, 2017
@jbrockmendel
Copy link
Member Author

Turns out get_value_box is necessary because it raises IndexError in cases where the other version ignores it. So now the change is just to move get_value_box from libts to libindex.

@codecov
Copy link

codecov bot commented Nov 20, 2017

Codecov Report

Merging #18371 into master will decrease coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #18371      +/-   ##
==========================================
- Coverage   91.38%   91.36%   -0.03%     
==========================================
  Files         164      164              
  Lines       49790    49799       +9     
==========================================
- Hits        45501    45499       -2     
- Misses       4289     4300      +11
Flag Coverage Δ
#multiple 89.16% <100%> (-0.01%) ⬇️
#single 39.55% <100%> (-0.04%) ⬇️
Impacted Files Coverage Δ
pandas/core/indexes/base.py 96.42% <100%> (ø) ⬆️
pandas/io/gbq.py 25% <0%> (-58.34%) ⬇️
pandas/core/indexes/interval.py 92.52% <0%> (-0.34%) ⬇️
pandas/core/frame.py 97.8% <0%> (-0.1%) ⬇️
pandas/core/internals.py 94.54% <0%> (ø) ⬆️
pandas/tseries/offsets.py 96.92% <0%> (+0.01%) ⬆️

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 b00e62c...38e29d1. Read the comment docs.



cpdef object get_value_box(ndarray arr, object loc):
cdef:
Copy link
Contributor

Choose a reason for hiding this comment

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

if you can add a doc string at some point

@jreback jreback added this to the 0.22.0 milestone Nov 20, 2017
@jreback jreback merged commit 7aa2737 into pandas-dev:master Nov 20, 2017
@jreback
Copy link
Contributor

jreback commented Nov 20, 2017

thanks!

@jorisvandenbossche
Copy link
Member

@jbrockmendel a general comment: can you use the 'CLN: ' tag in PR titles / commits for PRs that are purely cleaning things up? (so no bug, enhancement or performance related aspect)

@jbrockmendel jbrockmendel deleted the get_value_box branch December 8, 2017 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Clean Datetime Datetime data dtype Indexing Related to indexing on series/frames, not to indexes themselves

Projects

None yet

Development

Successfully merging this pull request may close these issues.

get_value_box duplication

3 participants