ENH: Fix for #1512, added StataReader and StataWriter to pandas.io.parsers #3270
Conversation
|
FYI, I have a student working on re-writing StataReader and StataWriter in Cython due to reports of their being slow. We might want to hold off on merging this until the rewrite. I don't know though up to pandas devs. |
|
@jseabold, Is there a timeline for that, it would help make a decision. I know there's an agreement for not having reverse deps from pandas on statsmodels, right now, the supported pandas IO file formats are all "vanilla" - csv, xls (well...), fixed width. |
The API is different for several reasons:
The way the parser works, however, is basically the same. I don't know if the performance of this parser somehow improved compared to old statsmodels parser. An improvement, however, is imaginable due to some small changes I did concerning the way data is written into a DataFrame. |
|
Should be done within the month. That's when the projects are due at least... This is a learning experience, so there might be some unforeseen stumbling blocks. FWIW, I plan to deprecate this from statsmodels given that it will be available in pandas, so we had planned on the Cython code making its way into pandas as well. Shouldn't be any need for a dependency. |
|
I should think this would belongs into pandas rather than statsmodels -- pure data I/O, nothing to do with statistics/econometrics except for the fact that handles the data format that is the near standard in econometrics. @y-p: I don't get the point about "vanilla" formats: sql, hdf5, ... Then there is the ability to read R dataframes, just seems to be in a different place. I cannot say too much about the statsmodels fork of the StataReader as I always worked with the original one (at some point, the statsmodels version converted everything into floats, which didn't work for me). The current version should keep the original data formats and adds some goodies in terms of carrying metadata along (I should add that PKEuS is my student). @jseabold: Happy to join forces here once you have produced something that gets rid of bottlenecks. I would guess that looping through the dataset would be the most crucial one. |
|
Stata files are a vendor-specific format for a proprietary stats package, I Don't think |
|
Foreign I/O seems very much to be a useful feature for pandas. It doesn't require any dependencies, and I'd find this very useful. I am using Stata -> pandas in many projects, and every time I wish it was in pandas. Cf. scipy.io for Matlab files. And in R |
|
If the same functionality is coming at the same time from two different sources aimed Would help if the two developers could reach a concensus re the two branches. |
|
My vote is merge this given it looks ok on your end. It doesn't look like the StataReader code is really much different than the statsmodels version, except the encoding, labels handling, and the use of Categorical (is this a correct reading of the changes?), so updating the optimization based on this won't be difficult. I can go ahead and deprecate the statsmodels version for our next release when I need to. It might be polite to acknowledge the original author Joe's work and my changes as well, though I'm not overly concerned about this. @PKEuS I noticed you removed the format checks. Did you test this on data created by early stata versions? I was never able to hunt down an old printed manual to see how the spec was different with ds_format < 113, but I was never able to read these datasets properly. |
No, they are still there. It will print a message if format is not 113, 114 or 115. See method StataReader._read_header() (line 2559).
Sure. Should I add this as a comment above the parser implementation? |
|
Ah, ok. I see it now. Wishful thinking. FWIW, I've been told that the old spec is published if you ever come across an old Stata manual lying around somewhere, so be on the lookout. There are a lot of old textbook example datasets that have not been updated. I usually see an section in the module docstring, but I don't know if it's worth it. I usually do it as a courtesy. I don't think there is anything in the license that says you have to. FWIW, looking back at my e-mail Joe changed the license of his original code to MIT, so we could include it in statsmodels. |
|
@jseabold: Do you have examples of old datasets floating around that don't work currently? At least for 113-115, it seems that only fields/formats were added, so it might be close to trivial to get previous ones to work. Will check whether we have old references at the University somewhere; in a first shot I could only get my hands on Stata 9 docs. |
|
I recall coming across them when I was going through textbook examples years ago, so some of the old stata press books or maybe Cameron and Travedi. You might find one here |
|
I am working on implementing support for Stata formats 104, 105 and 108 (that are the formats I found .dta files for in the Cameron and Travedi files). Does the license of this datasets allow using them as unit tests? |
|
@PKEuS Great! Re: inclusion, that's the rub with all these datasets. It's all very gray. I was never able to get Pravin Trivedi to respond to my requests. I don't see anywhere that I tried Colin Cameron. Maybe including them as test cases would be okay and not distributing them as examples/ data used anywhere else in the code, though IANAL. It also depends whether they originally compiled the data or if it came from another source. I usually try to track them back to an original author / agency and get express written permission. All data from US Government agencies is public domain. |
|
Reading old stata files works now, however, I didn't add unit tests for them so far. I could add tests without providing the files (just providing a link where to get them) and flag the tests as skip per default. What do you think? |
|
@PKEuS , that page's header seems to imply those files are there for use with stata. If any old stata file can be used as pass/fail but you have no compatibly-license test files, Is this merge-ready as far as you're concerned? |
|
Besides from missing unit tests for old formats, which I could add as SKIP-tests in an additional commits, I'd consider this to be merge ready. However, there might be small glitches for python2 compatibility which I don't know about, since I was only able to test this in a python3 environment. |
|
setup for Travis testing http://about.travis-ci.org/docs/user/getting-started/ step 2 then submit a new commit (or rebase) |
|
or |
|
I would pull all of this out of io.parsers and make a io.stata module (you can then have classes like Parser rather than StataParser), but that's optional |
|
@y-p I don't think there is a 'how to setup Travis' anywhere iin contributing/developers page ? |
|
I've enabled Travis now (I just enabled it in the Settings of my fork. yml file is there, so it probably just works now). I can commit the SKIP-test tomorrow evening, and then I'll see what travis says.
Creating a separate io.stata module wouldn't fit to the implementation of the other parsers, would it? Thus, I guess that such a refactorization should be done separately - there are other parsers like excel where it might also be a good idea to move it. But if you intend to shrink io.parsers, I can of course put the StataParsers into a separate file directly. |
|
@jreback , fixed. |
|
@PKEuS your have to turn it on it Travis too (login there) all of the various parsers have their own module |
|
I'll change the module. But the Excel parser seems to live in io.parsers - maybe a candidate to be moved out as well.
Thanks, I had forgotten that. |
|
@PKEuS 2 minor points
|
|
@PKEuS you are right about the parsers, I was thinking about the test files.....yes, I think excel should be moved out too...(different issue), actually its not so huge, you can leave if you want....I just think that different type of reader/writers should live in separate modules, but that's my 2c |
I agree. I just thought that all parsers live in io.parsers when I started porting the parser - but that was wrong assumption.
Yes, I'll fix that tomorrow |
|
@PKEuS great, start a trend! think excel should be refactored out too, will make an issue for that (its just more manageable esp when have reader /writers - no namespace collisions) |
|
Done |
|
@jreback , are you planning to merge this for 0.11? |
|
Ok by me. Is there a read_dta or read_stata in the main pandas namespace? Might consider it even though it's a bit of a niche function. Things like this and the DataReader stuff tend to get underutilized IMO because of a lack of visibility. |
|
There is a method DataFrame.from_dta() |
|
I'm -1 on adding new stuff after the RC and half an hour before the final release, |
|
I am not sure we are doing a 0.11.1......so....just tag experimental (its not imported by default anywhere right?) |
|
It is just imported in DataFrame.from_stata(), if that answers your question. |
|
we are doing a 0.11.1, and I think this'll wait until then before merged into master. |
|
@PKEuS can you rebase this on top of master....then can put it in 0.11.1, unless any objections (as I think this is pretty independent) |
|
also change release notes/whatsnew to v0.11.1 |
|
Done |
|
@PKEuS pls add a section in io.rst showing reading and writing (take a frame, write it, and read back in), to show how to use also can you rebase to a few number of commits (you can squash some), if possible |
|
Did you have any commits in mind for squashing? FWIW, I don't find the number of commits to be all that problematic at 9. It's clear to me what each one does in case I want to backport any changes to statsmodels while we're working on it. |
|
@jseabold oh...nothing specific, just try to squash whenver possible, but you have a valid point |
- Moved unit test data to tests/data - Added unit testing for old Stata formats (SKIP per default) - Use py3compat.PY3 instead of own implementation
- Don't call encode/decode on python2 - Added .dta type to setup.py - Fixed null byte
|
I squashed several bugfixing commits. io.rst is updated. |
|
thanks! this is great |
|
@PKEuS I made a small edit to the docs; discovered that on read-back the frame has a numeric index, and the index field, is that correct? take at look at the dev docs after 5pm today as well: http://pandas.pydata.org/pandas-docs/dev/io.html |
This pull request aims at fixing ticket #1512 and contains both a reader and a writer for Stata .dta files.
The code basically comes from th statsmodels project, however, I adapted it to the needs of pandas and implemented support for reading out stata value labels. The writer does not write those labels back.