Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ENH: Add StataWriter 118 for unicode support #30285

Merged
merged 3 commits into from
Dec 31, 2019

Conversation

bashtage
Copy link
Contributor

@bashtage bashtage commented Dec 16, 2019

Add StataWriter with unicode support

pandas/io/stata.py Outdated Show resolved Hide resolved
pandas/io/stata.py Outdated Show resolved Hide resolved
pandas/io/stata.py Outdated Show resolved Hide resolved
@jbrockmendel
Copy link
Member

@kshedden can you double-check this? It looks fine to me, but I think you're better-informed on this than I am.

@kshedden
Copy link
Contributor

Sorry, I'm not familiar at all with the writers.

@pep8speaks
Copy link

pep8speaks commented Dec 16, 2019

Hello @bashtage! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2019-12-31 16:38:17 UTC

@bashtage bashtage force-pushed the stata-118-writer branch 3 times, most recently from e1d47c4 to 3cd1088 Compare December 17, 2019 00:05
Copy link
Member

@WillAyd WillAyd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think looks OK generally. Would have been easier to review if the refactor / f-strings were done separate

pandas/io/stata.py Outdated Show resolved Hide resolved
pandas/io/stata.py Show resolved Hide resolved
pandas/io/stata.py Outdated Show resolved Hide resolved
Copy link
Contributor Author

@bashtage bashtage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think looks OK generally. Would have been easier to review if the refactor / f-strings were done separate

I'm sure this is true. Just binged on them a bit.

@bashtage bashtage requested review from WillAyd and jreback December 17, 2019 21:13
@bashtage
Copy link
Contributor Author

scikit-learn related failure

@jbrockmendel
Copy link
Member

LGTM. @TomAugspurger are we letting the sklearn thing in travis block merging?

@TomAugspurger
Copy link
Contributor

Nope, it's OK to ignore.

Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. very minor comments, pls rebase and ping on green.

pandas/io/stata.py Show resolved Hide resolved
pandas/io/stata.py Outdated Show resolved Hide resolved
pandas/io/stata.py Show resolved Hide resolved
pandas/io/stata.py Show resolved Hide resolved
Add StataWriter with unicode support
@TomAugspurger
Copy link
Contributor

TomAugspurger commented Dec 30, 2019

CI failure looked unrelated.

________________ TestRegistration.test_registering_no_warning _________________
[gw1] win32 -- Python 3.7.5 C:\Miniconda\envs\pandas-dev\python.exe

self = <pandas.tests.plotting.test_converter.TestRegistration object at 0x000002411CB02788>

    def test_registering_no_warning(self):
        plt = pytest.importorskip("matplotlib.pyplot")
        s = Series(range(12), index=date_range("2017", periods=12))
        _, ax = plt.subplots()
    
        # Set to the "warn" state, in case this isn't the first test run
        register_matplotlib_converters()
        with tm.assert_produces_warning(None) as w:
>           ax.plot(s.index, s.values)

Merged master to see if its fixed.

@jbrockmendel
Copy link
Member

CI failure again looks unrelated (and azure won't let me re-run). merged master to force re-run.

@bashtage
Copy link
Contributor Author

@jreback @jbrockmendel green.

@jbrockmendel jbrockmendel merged commit 35ba6b0 into pandas-dev:master Dec 31, 2019
@jbrockmendel
Copy link
Member

thanks @bashtage

hweecat pushed a commit to hweecat/pandas that referenced this pull request Jan 1, 2020
Add StataWriter with unicode support

Co-authored-by: Tom Augspurger <[email protected]>
Co-authored-by: jbrockmendel <[email protected]>
@bashtage bashtage deleted the stata-118-writer branch July 28, 2020 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement IO Stata read_stata, to_stata
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support writing unicode characters in df.to_stata()
8 participants