Read Stata version 118 files closes #9882

[kshedden]

closes #9882

This extends the Stata reader to handle version 118 (stata 14) format files.

It correctly handles the test file posted by @makmanalp. I don't have stata14 now to generate additional test files, but I'm not sure that it's necessary.

There is one point I'm not sure about, relating to the way that strls are indexed by a weird composite of a short and a 6 byte integer. The stata docs aren't clear on how this 8 byte composite should (or shouldn't) be byteswapped when the file and local endianness don't match. We currently byteswap it in this case (inheriting from our dta117 code) but I'm not sure that this is correct.

I can confirm that it works when the file and local machine are both little endian, which covers the most common situation.

I haven't looked at the writer much, it seems to always write version 114 files.

[jreback]

I think you should create a class for each version
(which have a hierarchy) - introspect the file and dispatch to the class for processing

should be simpler to maintain

[kshedden]

Having separate classes would work with read_stata but people also need to be able to create the class directly through init. If someone inits the class with StataReader('myfile.dta') I don't see how we can control which subclass gets created.

[jreback]

easy, StataReader is just a holder of the work class, which is instantiated at run-time. e.g. something like

class StataReader(...):

    def __init__(self, file....):

           if file_is_117:
                 self.data = V117(....)
           elif file_is_118:
                 self.data = V118(....)
           else:
                 raise ValueError("cannot parse this file")

you would simply change the impl and call methods on the worker class, while the external API would remain the same. The big advantage would be that this would leave all of the existing classes parsed the same; adding something new should be straightforward and minimal changes.

[kshedden]

I still don't get it. StataReader is either used as an iterator, or the user calls the read or data methods to obtain some or all of the data as a DataFrame. StataReader also has some additional methods and attributes like variable_labels whose implementation depends on the file version. If there were subclasses for different dta versions it seems to me that we would need to monkey patch the subclass methods into the base class (since the caller will obtain an instance of the base class not the correct subclass that matches the data file format). Also complicating matters is the fact that we have to read partway through the file before we know what the version is.

[jreback]

just trying to make it easier / more extensibile. It seems that every time this stata format changes this needs a big rewrite.

[kshedden]

I agree that it is sort of a mess. But the pieces that are version-specific are intertwined with code that works for all versions. The format changes are substantial as well, so to some degree the complexity is unavoidable.

[bashtage]

There is one point I'm not sure about, relating to the way that strls are indexed by a weird composite of a short and a 6 byte integer. The stata docs aren't clear on how this 8 byte composite should (or shouldn't) be byteswapped when the file and local endianness don't match. We currently byteswap it in this case (inheriting from our dta117 code) but I'm not sure that this is correct.

I don't think Stata runs on any big endian platforms, so the only tests that matter here are

1. Round trip on either
2. Generate on BE, read on LE
3. Generate on LE, read on BE

Of course with out a writer, the only one of these that will matter is #3.

[makmanalp]

Nice work! Unfortunately, I don't have access to a big-endian computer :(

[makmanalp]

Hey, is option b Ã\x9cnicode as intended?

[kshedden]

You are right I fixed this now.

[bashtage]

Is there a minimum Stata version that is supported? Some of the format info seems to be for very old, untested versions. This could be cleaned up.

[jreback]

more idiomatic to have something like this:

def f(typ):
      if typ <= 2045:
            return str(typ)
      try:
            return self.DTYPE_MAP_XML[typ]
       except KeyError:
            raise ValueError("cannot convert stata dtype [{0}]"
                            .format(typ))

self.dtyplist = map(f, typlist)

[jreback]

@kshedden I put a couple of examples of some code sharing that I think can be done (rather than inheriting from base classes, you can do it this way as well). To reduce the future maitenance burden.

[jreback]

use tm.assert_dict_equal

[jreback]

couple of minor comments. looks good though. after fix, pls squash/rebase. ping when green.

[kshedden]

@jreback -- this should be all set to go now.

[jreback]

@kshedden thanks! great effort on this!

[jorisvandenbossche]

I think this can still use an update in the documentation? (where they mention which versions read_stata supports: http://pandas.pydata.org/pandas-docs/stable/io.html#reading-from-stata-format) And maybe also a whatsnew note?

@kshedden can you do that in another PR?

[jorisvandenbossche]

Ah, I see @jreback already did the whatsnew :-)
But the docs can still use an update.

[jreback]

yes, @kshedden pls add a note (or section) on compatiblity (e.g. what versions are supported) in io.rst

[makmanalp]

🍰 Thanks to all involved - this makes my life way easier!