ENH: Added colspecs detection to read_fwf
State: closed

closes #4488

Implemented an algorithm that uses a bitmask to detect the gaps between the columns.
Also the reader buffers the lines used for detection in case it's input is not seek-able.
Added tests.


Labels:

Comments:
@alefnula pls add an extry in release notes and v0.13.0 example (and put the same example in io.rst). you can do these at the end....just an FYI

@jreback Yes, I was waiting for some feedback first. :)

@alefnula apparently my comment got lost: what happens if you have many many columns (e.g., 10,000) or a first row that is very long (maybe 100,000 characters or something)? Will your implementation still work?

@jtratner It will work. But it won't be very efficient if you have 10k columns. Characters lengths of the row are not such a big deal, I will allocate just one `ndarray` of `100.000` ints, which is 800k. That is the only structure I need... And the buffer. But I could remove the buffering if it could be guaranteed that the item passed to the reader is seekable.

Maybe the `PythonParser._make_reader` should take care of buffering...

@alefnula I think buffering is fine as well as what you are doing. The time you spend determining widths is a) much lower than human time doing it!, b) almost always less than actually reading the file....

what about doing an API like: `widths='infer'` to enable this? (I understand you then don't get specific rows/number of rows....but I view that as a real edge case)

@jreback As I said, whatever you like :) I would just use: `colspecs='infer'` since I'm actually inferring the colspecs...

Or maybe if either of the two is `'infer'`.

@alefnula how useful is to inspect certain rows do you think? (as opposed to just top 100)

@jreback At first I thought it could be useful, but when I started writing the tests I found out that it's much more painful to determine the correct rows from which the `colspecs` can be inferred than to actually count the column widths. So, now that I played with it a little, I think it's completely useless :D Except maybe in some really, really rare cases where one of rows determine the width.

But even then I'll inver correctly using this algorithm... When I added that parameter, I had another algorithm in mind, but it turned out that it wasn't correct.

ok in that case I would then turn auto detect on infer is passed for either widths or column spec

@jreback Deal. But that will wait have to wait until tomorrow, it's 2AM here :D

@jreback I would maybe just change one more thing: enable just the `colspecs` parameter to accept `'infer'`. It's duplicating if both parameters accept 'infer' and may confuse the user to think that we do something differently if he passes `widths='infer'` compared to `colspecs='infer'`.

Maybe even let it be the default if none of the above is specified?

@alefnula that's fine.....

@jreback Which part? Just the `colspecs` excepting `'infer'` or auto detection being the default fallback mechanism?

i like only accepting `'infer'` on `colspecs`.  I would also agree that if both `colspecs` and `width` are `None` then inferring is fine. (so basically the default for `colspecs` is then defaulted to `infer`. if for some reason the user sets `colspecs` to `None` then the existing error message can come up

Everything done. If I missed something or have to add something more please tell me.

@alefnula pls rebase and squash

@cpcloud comments?

@cpcloud @jtratner @y-p @wesm comments?

I think this is ok

@alefnula you need to add test cases for what happens if you _can't_ or _shouldn't be able to_ sniff the file (what happens, does it raise an error?) - there must exist files for which this doesn't work.  Are there any pathological cases where this sniffing could get confused? 

Also, please make sure you add a test case where you dynamically build a very long string (i.e., a very 'wide' file) and then test to make sure it can be sniffed appropriately.

also what happens with different delimiters (can the delimiter be a regular expression?), and unicode (UTF-8,  non-UTF-8, variable-width - you can look at some of the other tests for cases where the width of the letters vary, maybe try for some chinese characters?). 

also multicharacter delimiters, if those are accepted.

@jtratner OK so to recap:
1. Add `optional` to `colspecs` docs.
2. Move all `test*.fwf` files directly to `test_parser.py` as strings.
3. In the case of files that cannot be sniffed properly, you will just end up with two columns merged into one or one column separated into two. No exception will be thrown.
   For example:
   
   ```
   N    A          Name
   123foo   Joe Doe
   345bar   Joe Smith 
   455baz   Abe Lincoln
   ```
   
   Even though the header suggests there are three columns, the parser is not able to found the boundary between the first and the second. Also the third column will be split into two since first name and last name are clearly separated.
   If you have just one name with four letters:
   
   ```
   N    A          Name
   123foo   Joe Doe
   345bar   Joe Smith 
   455baz   Abe Lincoln
   333brb   Jack Lastname
   ```
   
   Then the parser will correctly parse the third column, because the clear separation is broken. (In this case it would be the second column since the first two cannot be separated).
4. About the very wide files. I tested this and it works just fine. The width of the file doesn't make any difference in parsing, because I'm not counting anything. It will just create a bigger one dimensional ndarray. I didn't thought it's necessary the test something that doesn't check any edge case or does anything differently. But I'll add that if you think it's necessary. No prob :)
5. Multiple characters delimiters will work fine, but strings and regexes wont. So telling the parser something like `delimiter='+~'` will slit the line on `+` and `~` characters. But telling the parser to split whenever it finds a `+-~` sequence wont. That didn't worked even before this change. Here is an untouched line of code from before that shows why:
   
   ``` python
   return [line[fromm:to].strip(self.delimiter)
           for (fromm, to) in self.colspecs]
   ```
   
   There is no regex `strip`, at least not a trivial one, so I think that this is just an unnecessary complication and shouldn't be implemented. So I'll just add tests that check for correctness of this.
6. Add tests that check unicode delimiters and unicode files.

Wide files - you don't need to add that, I just wanted to check that it worked. Somehow I thought that this was setup to accept regexes, but I think I was getting confused with csv's separator.

To confirm, you're saying that `read_fwf` with `'infer'` will _never_ raise an Exception, it will only produce poor results?

@jtratner Yes, that's correct. If it raises an exception it's a bug in my code.

okay, that all sounds fine. I do want to make sure you test with unicode - but you're working with string and not bytes anyways, so probably okay in Python 3. I don't have a great grasp on whether using something that's variable width unicode would mess up what you're doing.

@jtratner I'll add tests for that. So just to be clear. I should do the:

1 - Shorten and fix docs
2 - Move files directly to tests.
5 - Add more tests for multi character delimiters.
6 - Add more tests for variable width unicode.

@alefnula Happy to clarify.
1. Yes.
2. Yes.

`5.` If you already have a test for multi character delimiters, not necessary to add more. (I must have missed it).  Please do add a test that uses a variable-width unicode delimiter though.
`6.` Yes.

Thanks!

@jtratner All done.

@alefnula just need a rebase :)

@cpcloud I rebased and squashed it. It's just one commit. Or you mean something else?