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

PERF: Cython version of Python _TIMEPAT regexp in parsing.pyx #26204

Merged
merged 3 commits into from Apr 24, 2019

Conversation

anmyachev
Copy link
Contributor

  • closes #N/A
  • tests added / passed
  • passes git diff upstream/master -u -- "*.py" | flake8 --diff
  • whatsnew entry

asv continuous -f 1.05 origin/master HEAD -b ^io.csv.ReadCSVParseSpecialDate -a warmup_time=2 -a sample_time=2:

master patch ratio test_name
276±1ms 262±2ms 0.95 io.csv.ReadCSVParseSpecialDate.time_read_special_date('hm')
16.6±0.2ms 14.2±0.08ms 0.85 io.csv.ReadCSVParseSpecialDate.time_read_special_date('mdY')
42.7±0.9ms 35.0±0.2ms 0.82 io.csv.ReadCSVParseSpecialDate.time_read_special_date('mY')

@codecov
Copy link

codecov bot commented Apr 24, 2019

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #26204      +/-   ##
==========================================
- Coverage   91.98%   91.98%   -0.01%     
==========================================
  Files         175      175              
  Lines       52372    52372              
==========================================
- Hits        48176    48173       -3     
- Misses       4196     4199       +3
Flag Coverage Δ
#multiple 90.53% <ø> (ø) ⬆️
#single 40.7% <ø> (-0.16%) ⬇️
Impacted Files Coverage Δ
pandas/io/gbq.py 78.94% <0%> (-10.53%) ⬇️
pandas/core/frame.py 96.9% <0%> (-0.12%) ⬇️
pandas/util/testing.py 90.71% <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 d74901b...8f26757. Read the comment docs.

@codecov
Copy link

codecov bot commented Apr 24, 2019

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #26204      +/-   ##
==========================================
- Coverage   91.98%   91.98%   -0.01%     
==========================================
  Files         175      175              
  Lines       52372    52372              
==========================================
- Hits        48176    48172       -4     
- Misses       4196     4200       +4
Flag Coverage Δ
#multiple 90.53% <ø> (ø) ⬆️
#single 40.7% <ø> (-0.15%) ⬇️
Impacted Files Coverage Δ
pandas/io/gbq.py 78.94% <0%> (-10.53%) ⬇️
pandas/core/frame.py 96.9% <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 d41c1da...464380f. Read the comment docs.

@vnlitvinov
Copy link
Contributor

Note to reviewers - current failures seem to be introduced by #26200

@jreback jreback added the Performance Memory or execution speed performance label Apr 24, 2019
@jreback jreback added this to the 0.25.0 milestone Apr 24, 2019
Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

otherwise lgtm.

@@ -144,6 +142,26 @@ cdef inline object _parse_delimited_date(object date_string, bint dayfirst):
raise DateParseError("Invalid date specified ({}/{})".format(month, day))


cdef inline bint does_string_look_like_time(object parse_string):
cdef:
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add a doc-string here

Copy link
Contributor

Choose a reason for hiding this comment

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

docstring added

@jreback
Copy link
Contributor

jreback commented Apr 24, 2019

also merge master; ci has been fixed

@vnlitvinov
Copy link
Contributor

also merge master; ci has been fixed

I've rebased instead, I personally find rebased history cleaner than merged.

@jreback
Copy link
Contributor

jreback commented Apr 24, 2019

I've rebased instead, I personally find rebased history cleaner than merged.

I agree :-D

we use merge master as its somwhat friendlier to new users, in any event everything is squashed on merge anyways (so the commit history is only relevant to you)

@vnlitvinov
Copy link
Contributor

@jreback pinging on green

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

jreback commented Apr 24, 2019

thanks @anmyachev

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Performance Memory or execution speed performance Timeseries
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants