Skip to content

Conversation

anmyachev
Copy link
Contributor

@anmyachev anmyachev commented Mar 29, 2019

@codecov
Copy link

codecov bot commented Mar 29, 2019

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #25925      +/-   ##
==========================================
- Coverage    91.8%   91.79%   -0.01%     
==========================================
  Files         174      174              
  Lines       52536    52536              
==========================================
- Hits        48231    48226       -5     
- Misses       4305     4310       +5
Flag Coverage Δ
#multiple 90.35% <ø> (ø) ⬆️
#single 41.88% <ø> (-0.07%) ⬇️
Impacted Files Coverage Δ
pandas/io/gbq.py 75% <0%> (-12.5%) ⬇️
pandas/core/frame.py 96.79% <0%> (-0.12%) ⬇️
pandas/util/testing.py 90.63% <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 1d4c89f...d7c6ffa. Read the comment docs.

@codecov
Copy link

codecov bot commented Mar 29, 2019

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #25925      +/-   ##
==========================================
- Coverage   91.81%    91.8%   -0.01%     
==========================================
  Files         175      175              
  Lines       52578    52578              
==========================================
- Hits        48273    48269       -4     
- Misses       4305     4309       +4
Flag Coverage Δ
#multiple 90.36% <ø> (ø) ⬆️
#single 41.9% <ø> (-0.08%) ⬇️
Impacted Files Coverage Δ
pandas/io/gbq.py 75% <0%> (-12.5%) ⬇️
pandas/core/frame.py 96.79% <0%> (-0.12%) ⬇️

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 f0ba498...7d56a15. Read the comment docs.

@gfyoung gfyoung added Numeric Operations Arithmetic, Comparison, and Logical operations Internals Related to non-user accessible pandas implementation Clean labels Mar 30, 2019
@gfyoung gfyoung requested a review from jbrockmendel March 30, 2019 01:44
@gfyoung
Copy link
Member

gfyoung commented Mar 30, 2019

@anmyachev : Thanks for the PR!

Can you run asv benchmarks to see if performance is impacted with this de-duplication?

@jbrockmendel
Copy link
Member

AFAICT this looks fine, but I'm going to kick the review over to @chris-b1 who is better-versed in this area.

@gfyoung
Copy link
Member

gfyoung commented Mar 30, 2019

@jbrockmendel: Sounds good. Was just pinging for the modified dependency linking (given your thoughts interest in reorganizing internal dependencies to make it more DAG)

@anmyachev
Copy link
Contributor Author

@gfyoung after this run:
asv continuous -f 1.01 origin/master HEAD -e -b inference.MaybeConvertNumeric
result: BENCHMARKS NOT SIGNIFICANTLY CHANGED.
Same result for run asv continuous -f 1.01 origin/master HEAD -e -b io.csv

  • First run check perf of xstrtod with maybe_int != NULL
  • Second run check perf of xstrtod with maybe_int == NULL

@anmyachev anmyachev force-pushed the concat_xstrtod_implementations branch from ff2ae8c to 7d56a15 Compare March 30, 2019 20:08
@jreback
Copy link
Contributor

jreback commented Apr 3, 2019

@chris-b1 any comments here

@jreback jreback added this to the 0.25.0 milestone Apr 3, 2019
Copy link
Contributor

@chris-b1 chris-b1 left a comment

Choose a reason for hiding this comment

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

lgtm!

@jreback jreback merged commit d591ade into pandas-dev:master Apr 3, 2019
@jreback
Copy link
Contributor

jreback commented Apr 3, 2019

thanks @anmyachev

@anmyachev anmyachev deleted the concat_xstrtod_implementations branch April 4, 2019 08:12
anmyachev added a commit to anmyachev/pandas that referenced this pull request Apr 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Clean Internals Related to non-user accessible pandas implementation Numeric Operations Arithmetic, Comparison, and Logical operations

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Duplicate xstrtod implementations

5 participants