DOC: #22896, Fixed docstring of to_stata in pandas/core/frame.py #23152

ryankarlos · 2018-10-14T17:41:46Z

closes DOC: Fix the docstring of to_stata in pandas/core/frame.py #22896
tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
passes ./scripts/validate_docstrings.py pandas.DataFrame.to_stata

pep8speaks · 2018-10-14T17:41:51Z

Hello @ryankarlos! Thanks for updating the PR.

There are no PEP8 issues in the file pandas/core/frame.py !

Comment last updated on November 20, 2018 at 21:28 Hours UTC

ryankarlos · 2018-10-14T18:05:33Z

Ive followed the docstring format here https://pandas.pydata.org/pandas-docs/stable/contributing_docstring.html

Ive added in a returns section for to_stata in pandas/core/frame.py, corrected linting issues and modified the examples as some of the variables (mainly for the date example) were not defined and hence failing some of the tests.

For some reason, the tests were throwing up linting errors for an unrelated older pull request - but Ive corrected these as well.

datapythonista · 2018-10-14T18:37:05Z

Thanks for the work on this @ryankarlos

Can you leave in this PR just the changes in the to_stata docstring please? Things become more complex if we mix things, and I see some blank lines added in unrelated code, and changes to the split function (which are welcome, but in a separate PR).

I also see that there are PEP-8 issues in the examples, and may be they can be simplified a bit.

codecov · 2018-10-14T18:38:53Z

Codecov Report

Merging #23152 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master   #23152   +/-   ##
=======================================
  Coverage   92.29%   92.29%           
=======================================
  Files         161      161           
  Lines       51493    51493           
=======================================
  Hits        47524    47524           
  Misses       3969     3969

Flag	Coverage Δ
#multiple	`90.68% <ø> (ø)`	⬆️
#single	`42.32% <ø> (ø)`	⬆️

Impacted Files	Coverage Δ
pandas/core/frame.py	`97.02% <ø> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 99df7da...64e02d0. Read the comment docs.

datapythonista · 2018-10-14T18:43:50Z

pandas/core/frame.py

+        >>> dates_1 = pd.to_datetime(["2016-05-06","2017-05-07", "2018-05-10"])
+        >>> dates_2 = pd.to_datetime(["2016-11-06","2017-11-07", "2018-11-10"])
+        >>> df_dates = pd.DataFrame({'date1': dates_1, 'date2': dates_2})
+        >>> df_dates.to_stata('./date_data_file.dta', {2 : 'tw'})


Can you explain what the {2: 'tw'} means? I know this is part of the original docstring, but I think we should make the examples in a way that they explain what is going on. I don't know much about stata or this function, and this without explanation is not useful to me.

Also, would be great to simplify this. May be just a date column if that makes sense, without creating variables first, and may be using date_range to avoid so much information that does not add value. And if we can create a single DataFrame at the beginning that is useful for all functions, that would be much better.

Finally, if we can use something looking more like a real example. I think it gives a better impression and helps understand than a [1, 2, 3]. We've got some examples with animals in pandas/core/generic.py, I'd use one of them.

The tw is a Stata specific option for displaying time in weeks i believe - i have updated it with a single data frame using the animals examples from pandas/core/generic.py and excluded the separate example specifically for dates (i agree it does not really add any value).

datapythonista · 2018-10-14T18:44:56Z

pandas/core/frame.py


        Alternatively you can create an instance of the StataWriter class

-        >>> writer = StataWriter('./data_file.dta', data)
+        >>> StataWriter = pd.io.stata.StataWriter
+        >>> writer = StataWriter('./data_file.dta', df)


I'd use instead (assuming you're using the animals data):
>>> writer = pd.io.stata.StataWriter('animals.dta', df)

ryankarlos · 2018-10-14T22:49:48Z

Thanks for the work on this @ryankarlos

Can you leave in this PR just the changes in the to_stata docstring please? Things become more complex if we mix things, and I see some blank lines added in unrelated code, and changes to the split function (which are welcome, but in a separate PR).

I also see that there are PEP-8 issues in the examples, and may be they can be simplified a bit.

@datapythonista Sorry for that, I have undone the changes to the other docstring and only left the to_stata one. I wasn't sure what those PEP-8 issues regarding the escape sequence were referring to (as they didn't show up in the flake8 test )- I have modified and simplified the examples and include - instead of \ between the days, month and year .... seems like that may have fixed the problem.

ryankarlos · 2018-10-20T02:43:55Z

@datapythonista let me know if there are any updates required

datapythonista · 2018-10-24T12:58:51Z

Looks good, added some comments.

ryankarlos · 2018-10-24T21:15:28Z

@datapythonista Did you add comments to the latest commit - as I cannot find anything ?

datapythonista

Sorry, I don't have a computer right now, and it's tricky to do the reviews from the phone. I think the comments should be added now.

datapythonista · 2018-10-24T12:49:04Z

pandas/core/frame.py

@@ -1846,12 +1846,15 @@ def to_stata(self, fname, convert_dates=None, write_index=True,
                 data_label=None, variable_labels=None, version=114,
                 convert_strl=None):
        """
-        Export Stata binary dta files.
+        Converting data frame object to Stata dta format.


I'd use DataFrame instead of data frame.

And dta between quotes to be consistent with the next paragraph (or probably better to remove them there).

datapythonista · 2018-10-24T12:49:21Z

pandas/core/frame.py

-        Export Stata binary dta files.
+        Converting data frame object to Stata dta format.
+
+        Writes the Dataframe to a Stata dataset file.


DataFrame with capital F

datapythonista · 2018-10-24T12:51:23Z

pandas/core/frame.py

+        Converting data frame object to Stata dta format.
+
+        Writes the Dataframe to a Stata dataset file.
+        "dta" files contain a Stata dataset.

        Parameters
        ----------
        fname : path (string), buffer or path object


The types in this line should be Python types. I think in this case should be: str, file descriptor or pathlib.Path

datapythonista · 2018-10-24T12:54:23Z

pandas/core/frame.py

@@ -1896,6 +1899,10 @@ def to_stata(self, fname, convert_dates=None, write_index=True,

            .. versionadded:: 0.23.0

+        Returns
+        ------
+        Stata (dta) file or StataWriter.


This should be the Python returned type of the function. I don't think it returns anything. If this is the case just remove the section.

datapythonista · 2018-10-24T12:58:05Z

pandas/core/frame.py


        Examples
        --------
-        >>> data.to_stata('./data_file.dta')
+        Converting dataframe with date column to Stata dta file
+        using the to_stata method.


No idea about stata, but I don't think having a date column and indices adds any value here unless we can explain why.

I'd use a much simpler example. And I'd leave the StataWriter example to that docstring.

ryankarlos · 2018-10-25T23:18:34Z

@datapythonista Thanks for the comments - incorporated the changes.

datapythonista

Sorry @ryankarlos, I've just seen that I didn't submit the comments of the last review (I did it from the phone, and it's tricky).

Besides those, can you edit ci/code_checks.py and in the part of the doctests, stop excluding this docstring, as they should now pass.

Thanks!

datapythonista · 2018-10-25T23:34:44Z

pandas/core/frame.py

        ...                               'parrot'],
-        ...                    'speed': [350, 18, 361, 15]}).set_index(['date',
-        ...                                                    'animal'])
+        ...                    'speed': [350, 18, 361, 15]})
        >>> df.to_stata('animals.dta')


can you add # doctest: +SKIP so the test is not run, and the file is not generated and left in the disk.

datapythonista · 2018-10-25T23:35:40Z

pandas/core/frame.py

-            then the buffer will not be automatically closed after the file
-            data has been written.
+        fname : str, file descriptor or pathlib.Path
+            String or path to file which needs to be converted.


I like the original description more.

datapythonista · 2018-10-25T23:36:51Z

pandas/core/frame.py

-        pandas.io.stata.StataWriter117 : low-level writer for version 117 files
+        pandas.read_stata : Import Stata data files.
+        pandas.io.stata.StataWriter : Writer for Stata data files.
+        pandas.io.stata.StataWriter117 : Writer for version 117 files.


The pandas. prefix is unnecessary.

datapythonista · 2018-10-25T23:37:48Z

pandas/core/frame.py

-        >>> writer.write_file()
-
-        With dates:
+        Converting DataFrame to Stata "dta" file using the to_stata method.


I think this line is obvious given the context, I'd remove it.

ryankarlos · 2018-11-06T00:53:18Z

@datapythonista added requested changes

datapythonista

Looks good. Can you add optional to the data_label parameter, and the default to version : {114, 117}, default 114.

I think with that is ready to be merged (if you can also take a look at the CI and make sure it's green, so can merge).

ryankarlos · 2018-11-08T13:10:35Z

@datapythonista Thanks, changes added - all green now

datapythonista · 2018-11-09T12:13:08Z

lgtm, just changed a word. I'll wait for for someone else to merge this, so we have an extra pair of eyes on it.

Thanks @ryankarlos

jreback

tiny comments. @bashtage if you can have a look.

pandas/core/frame.py

ryankarlos · 2018-11-20T23:39:29Z

@jreback All green now

jreback · 2018-11-21T12:41:00Z

thanks @ryankarlos

…me.py (pandas-dev#23152)

ryankarlos changed the title ~~To stata docstring~~ DOC: #22896, Fixed docstring of to_stata in pandas/core/frame.py Oct 14, 2018

datapythonista added Docs IO Stata read_stata, to_stata labels Oct 14, 2018

datapythonista reviewed Oct 14, 2018

View reviewed changes

ryankarlos closed this Oct 14, 2018

ryankarlos force-pushed the to_stata_docstring branch from 2016051 to a0ca4d7 Compare October 14, 2018 22:16

ryankarlos reopened this Oct 14, 2018

datapythonista reviewed Oct 25, 2018

View reviewed changes

datapythonista reviewed Nov 4, 2018

View reviewed changes

ryankarlos mentioned this pull request Nov 5, 2018

DOC: Fixing test and validate docstrings for section ordering #23245

Closed

3 tasks

ryankarlos force-pushed the to_stata_docstring branch from 2e4c2cb to 42fec06 Compare November 6, 2018 00:51

datapythonista approved these changes Nov 6, 2018

View reviewed changes

ryankarlos force-pushed the to_stata_docstring branch from 42fec06 to 994f0b9 Compare November 8, 2018 00:13

jreback added this to the 0.24.0 milestone Nov 20, 2018

jreback requested changes Nov 20, 2018

View reviewed changes

pandas/core/frame.py Outdated Show resolved Hide resolved

ryankarlos added 3 commits November 20, 2018 21:16

Updated examples

860f474

Add returns section

1b7bac4

Fix whitespace on blank line 1918:1: W293

85b7c20

ryankarlos added 4 commits November 20, 2018 21:16

Incorporated comments

13ad994

Minor changes

8697ea8

Final

dad93b9

Final fixes

64e02d0

ryankarlos force-pushed the to_stata_docstring branch from d67588e to 64e02d0 Compare November 20, 2018 21:28

jreback approved these changes Nov 21, 2018

View reviewed changes

jreback merged commit 8341cec into pandas-dev:master Nov 21, 2018

Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019

DOC: pandas-dev#22896, Fixed docstring of to_stata in pandas/core/fra…

a002e87

…me.py (pandas-dev#23152)

Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019

DOC: pandas-dev#22896, Fixed docstring of to_stata in pandas/core/fra…

3a9c767

…me.py (pandas-dev#23152)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DOC: #22896, Fixed docstring of to_stata in pandas/core/frame.py #23152

DOC: #22896, Fixed docstring of to_stata in pandas/core/frame.py #23152

ryankarlos commented Oct 14, 2018 •

edited

Loading

pep8speaks commented Oct 14, 2018 •

edited

Loading

ryankarlos commented Oct 14, 2018 •

edited

Loading

datapythonista commented Oct 14, 2018

codecov bot commented Oct 14, 2018 •

edited

Loading

datapythonista Oct 14, 2018

ryankarlos Oct 14, 2018

datapythonista Oct 14, 2018

ryankarlos commented Oct 14, 2018 •

edited

Loading

ryankarlos commented Oct 20, 2018

datapythonista commented Oct 24, 2018

ryankarlos commented Oct 24, 2018

datapythonista left a comment

datapythonista Oct 24, 2018

datapythonista Oct 24, 2018

datapythonista Oct 24, 2018

datapythonista Oct 24, 2018

datapythonista Oct 24, 2018

ryankarlos commented Oct 25, 2018 •

edited

Loading

datapythonista left a comment

datapythonista Oct 25, 2018

datapythonista Oct 25, 2018

datapythonista Oct 25, 2018

datapythonista Oct 25, 2018

ryankarlos commented Nov 6, 2018

datapythonista left a comment

ryankarlos commented Nov 8, 2018

datapythonista commented Nov 9, 2018

jreback left a comment

ryankarlos commented Nov 20, 2018

jreback commented Nov 21, 2018

DOC: #22896, Fixed docstring of to_stata in pandas/core/frame.py #23152

DOC: #22896, Fixed docstring of to_stata in pandas/core/frame.py #23152

Conversation

ryankarlos commented Oct 14, 2018 • edited Loading

pep8speaks commented Oct 14, 2018 • edited Loading

Comment last updated on November 20, 2018 at 21:28 Hours UTC

ryankarlos commented Oct 14, 2018 • edited Loading

datapythonista commented Oct 14, 2018

codecov bot commented Oct 14, 2018 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ryankarlos commented Oct 14, 2018 • edited Loading

ryankarlos commented Oct 20, 2018

datapythonista commented Oct 24, 2018

ryankarlos commented Oct 24, 2018

datapythonista left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ryankarlos commented Oct 25, 2018 • edited Loading

datapythonista left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ryankarlos commented Nov 6, 2018

datapythonista left a comment

Choose a reason for hiding this comment

ryankarlos commented Nov 8, 2018

datapythonista commented Nov 9, 2018

jreback left a comment

Choose a reason for hiding this comment

ryankarlos commented Nov 20, 2018

jreback commented Nov 21, 2018

ryankarlos commented Oct 14, 2018 •

edited

Loading

pep8speaks commented Oct 14, 2018 •

edited

Loading

ryankarlos commented Oct 14, 2018 •

edited

Loading

codecov bot commented Oct 14, 2018 •

edited

Loading

ryankarlos commented Oct 14, 2018 •

edited

Loading

ryankarlos commented Oct 25, 2018 •

edited

Loading