ENH: Treat 'Inf' as infinity in text parser

[jtratner]

Makes 'inf' checks case insensitive using strcasecmp
Fixes #4219.

[jreback]

was using strcasecmp slower? (prob not a big deal either way)

[jtratner]

No. I'm doing that instead. Just found a few other things that needed to be fixed (plus means test cases need to be larger).

Is TestPythonParser supposed to call the parser functions in parsers.pyx?

[jreback]

some of python parser functions are shared

but not sure (your tests are run thru both parsers though so u will know if they work or not)

[jtratner]

Okay, I guess I thought engine='python' would set it to pure python only. Not a big deal to me, just wanted to make sure (thought it could have been a bug or something)

[jtratner]

And here's a perf test between various options: http://nbviewer.ipython.org/5980552

[jreback]

@jtratner you are very thorough!

and you said u were busy this week :)

[jtratner]

haha ... didn't take long to do so I figured I'd crank it out.

This means that any text that converts to 'inf' when lower cased is considered infinity [though only when attempting to convert column to float/double]. Is that what we want to do?

(e.g., "-iNF", "inf", "inF" all would become versions of +/- infinity)

[jreback]

yes I think any of those variations are fine

it might be easier/faster to make a hash table of all of the combinations and just do a lookup
(this is how na values r done) - but only if u r interested in trying it out - prob not necessary

[jtratner]

hmm...I'll think about it. Could probably just reuse the function that creates the hashtable for na_values then. But given that right now it's a 25ish character change in the actual code (import strcasecmp, strcmp-->strcasecmp), this is pretty simple. [just realized I left in the extra (and now unnecessary inf values)]

[jreback]

yep looks good to go

[jtratner]

If you want doc update on this, I can add Monday.

On Thu, Jul 11, 2013 at 10:13 PM, jreback [email protected] wrote:

yep looks good to go

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/4220#issuecomment-20854752
.

[jreback]

hold off on release notes till after 0.12...have to add the new file

[jtratner]

Will do.

[jtratner]

Btw - when are you pulling the trigger on 0.12? :)

[jreback]

just a couple minor issues with 0.12rc1...then should be out

[jtratner]

@jreback This should be ready to go (just waitin' on Travis).

[jreback]

should 'infinity' be there too?

[jtratner]

I'd say no, because that's bordering more on things that could be string-like (not 100% sure: are inf checks are done before dtype decision?). If we did that, would need to switch to hashtable format, which I could do. your call on how you want to do it.

[jreback]

right. so forget that
looks good what u have

can u just do a quick perf test
just run vs 0.12 vbenchs
you can use -r regex to only run some
I some in the io_brench

just to make sure it doesn't slow it down much (I think it'll be fine though)

[jtratner]

@jreback is there a way to collect vbench results into a csv?

I find the my results can vary a lot and being able to combine multiple would help smooth it out.

[jtratner]

Here's perf results - ran it twice (wasn't sure what regex to use):
https://gist.github.com/jtratner/cb114164d3bdce2e4dfa

That said, reading-related items don't seem to be particularly affected (or at least are not negative).

[cpcloud]

Make sure you're not doing literally anything else when you run test perf. I sometimes forget that and I get "bad" results. Ratio > 1.15 is usually something sketchy but only if you weren't running anything else.

[cpcloud]

Well anything like building docs or compiling stuff. Surfing the web is OK.

[jtratner]

Here are just the "read_*" benches:

*** Processing results...
***
Invoked with :
--ncalls: 3
--repeats: 3


-------------------------------------------------------------------------------
Test name                                    | head[ms] | base[ms] |  ratio   |
-------------------------------------------------------------------------------
read_store_table                             |   3.3030 |   4.4127 |   0.7485 |
read_csv_vb                                  |  29.5163 |  37.5853 |   0.7853 |
read_csv_comment2                            |  29.8669 |  31.6617 |   0.9433 |
read_table_multiple_date                     | 295.3293 | 298.5720 |   0.9891 |
read_store                                   |   2.6910 |   2.6627 |   1.0107 |
read_table_multiple_date_baseline            | 137.8644 | 135.1160 |   1.0203 |
read_store_table_mixed                       |   6.5330 |   6.3693 |   1.0257 |
read_parse_dates_iso8601                     |   1.7890 |   1.7393 |   1.0286 |
read_store_mixed                             |   5.8227 |   5.6427 |   1.0319 |
read_csv_thou_vb                             |  43.4330 |  41.5463 |   1.0454 |
read_store_table_panel                       |  42.0457 |  40.0033 |   1.0511 |
read_store_table_wide                        |  21.0853 |  19.6953 |   1.0706 |
read_csv_standard                            |  18.4487 |  15.5730 |   1.1847 |
-------------------------------------------------------------------------------
Test name                                    | head[ms] | base[ms] |  ratio   |
-------------------------------------------------------------------------------

Ratio < 1.0 means the target commit is faster then the baseline.
Seed used: 1234

Target [3080721] : ENH: Use case-insensitive checks for 'inf' in parser.
Base   [6c8fca9] : Merge pull request #4282 from cpcloud/add-clipboard-test

[jtratner]

I just ran the bench against a bogus commit (literally adding whitespace to ez_setup.py) and I get about the same differences just by random chance, so I'm not convinced that these results are particularly meaningful.

[jtratner]

e.g. here's against a basically blank commit:

-------------------------------------------------------------------------------
Test name                                    | head[ms] | base[ms] |  ratio   |
-------------------------------------------------------------------------------
read_store_table_mixed                       |   6.5234 |   6.8897 |   0.9468 |
read_store_table_panel                       |  40.2050 |  41.2770 |   0.9740 |
read_store                                   |   2.7483 |   2.7854 |   0.9867 |
read_csv_standard                            |  15.7667 |  15.9566 |   0.9881 |
read_csv_comment2                            |  32.8847 |  33.2080 |   0.9903 |
read_store_table                             |   3.2610 |   3.2636 |   0.9992 |
read_store_mixed                             |   5.7760 |   5.6917 |   1.0148 |
read_csv_thou_vb                             |  43.5576 |  42.8537 |   1.0164 |
read_parse_dates_iso8601                     |   1.7897 |   1.7587 |   1.0177 |
read_store_table_wide                        |  20.0947 |  19.7203 |   1.0190 |
read_table_multiple_date_baseline            | 141.1477 | 137.1057 |   1.0295 |
read_table_multiple_date                     | 317.5010 | 296.3533 |   1.0714 |
read_csv_vb                                  |  34.2953 |  29.2070 |   1.1742 |
-------------------------------------------------------------------------------
Test name                                    | head[ms] | base[ms] |  ratio   |
-------------------------------------------------------------------------------

[jreback]

perf testing is actually quite tricky as many things can affect it from one run to another - just looking to make sure that the change which we think is a non event really is

so looks good then

[jreback]

thank you sir!