Adds string subclass check to dataframe.__getitem__

[jrbourbeau]

This PR adds string subclass support to DataFrame.__getitem__

Fixes #3297

• Tests added / passed
• Passes flake8 dask
• Fully documented, including docs/source/changelog.rst for all changes
  and one of the docs/source/*-api.rst files for new API

[mrocklin]

This seems fine to me.

It's odd seeing six in the codebase, which hasn't been used historically and isn't an explicit dependency. I notice though that it has been used once in dataframe/io/sql.py for the last year though. Thoughts on if we should add it as an explicit dependency of dask[dataframe] @jakirkham or work around with our own compatibility.py file?

[mrocklin]

Thanks for taking this on @jrbourbeau !

It's really nice to see people who solve other peoples' problems :)

[jrbourbeau]

Thanks @mrocklin!

[jakirkham]

Have no qualms with six other than it's style is a bit dated. Personally would encourage using a more modern Python 2/3 compat library like python-future or eight. Using our own compat module would be an option.

[jrbourbeau]

Good to know about string_types in compatibility.py, I hadn't run across that before! I've got no preference re: six and can switch to using dask.compatibility.string_types here.

[mrocklin]

It looks like we have string_types already in compatibility.py.

@jrbourbeau I recommend using this so that we can just avoid the question altogether.

If you felt like fixing the use of six in dask/dataframe/io/sql.py as well that would be welcome :)

[mrocklin]

Ah, you beat me to the comment

[jrbourbeau]

Getting some OSError: [Errno 12] Cannot allocate memory errors for one of the builds on Travis. Should be unrelated to the changes made in this PR.

[mrocklin]

Hrm, that's interesting. I agree that it's likely unrelated. Restarting. We'll see if it recurs.

…

On Wed, May 2, 2018 at 5:43 PM, James Bourbeau ***@***.***> wrote: Getting some OSError: [Errno 12] Cannot allocate memory errors for one of the builds on Travis. Should be unrelated to the changes made in this PR. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3461 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AASszEByXPa5YMFT1tdpalltqUdgOFc1ks5tuiiRgaJpZM4TwDKn> .

[mrocklin]

This looks good to me. Thanks @jrbourbeau !

[jrbourbeau]

I guess those were just spurious errors

[mrocklin]

Merged! Thanks @jrbourbeau !