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

CLN: DataFrame move doc from __init__ to cls #3342

Closed
wants to merge 2 commits into from

Conversation

dengemann
Copy link
Contributor

This, in part addresses our discussion from #3337.
This PR moves the doc string from init to the more common class location.
(cf.. numpy, rest of pandas classes that have a doc string)
Besides STY / consistency this would allow for more directly accessing the doc string in a debugging (pdb) session:

print mydf.__doc__

This commit moves the doc string to a more common location.
@ghost
Copy link

ghost commented Apr 13, 2013

Thanks. For consistency's sake, we'll hold off on this until the rest of the major classes
get the same treatment: Series, Panel, *Index...

@dengemann
Copy link
Contributor Author

@y-p one approximation to what is to be regarded as 'major classes' might be looking what we find in the pandas name space:

These are the classes for which doc is not directly under the class statement, below the rest of the classes.

In [11]: [(k, c.__doc__) for k, c in pd.__dict__.items() if k[0].isupper() and c.__doc__ is None]
Out[11]: 
[('WidePanel', None),
 ('SparseTimeSeries', None),
 ('Panel4D', None),
 ('SparseSeries', None),
 ('Timestamp', None),
 ('Period', None),
 ('Series', None),
 ('DateRange', None),
 ('NaT', None),
 ('Panel', None),
 ('TimeSeries', None),
 ('Int64Index', None)]

# here the ones for which __doc__ is under the class statement
In [12]: [k for k, c in pd.__dict__.items() if k[0].isupper() and c.__doc__ is not None]
Out[12]: 
['SparseArray',
 'Categorical',
 'ExcelWriter',
 'DataFrame',
 'Index',
 'SparseList',
 'PeriodIndex',
 'Factor',
 'SparseDataFrame',
 'Term',
 'MultiIndex',
 'HDFStore',
 'DateOffset',
 'SparsePanel',
 'ExcelFile',
 'TimeGrouper',
 'DatetimeIndex']

I could do the same for the missing cases listed above. Wdyt?

@ghost
Copy link

ghost commented Apr 13, 2013

Yeah, that'd be good.
That's pretty inconsistent alright.

@dengemann
Copy link
Contributor Author

The remaining cases are such that constitute pure factory classes employing new or have entirely missing doc strings.
Would it make to add empty doc strings in such cases?

In [6]: sorted([(c.__module__, k) for k, c in pd.__dict__.items() if k[0].isupper() and c.__doc__ is None])
Out[6]: 
[('pandas.core.daterange', 'DateRange'),
 ('pandas.core.index', 'Int64Index'),
 ('pandas.core.panelnd', 'Panel4D'),
 ('pandas.core.series', 'TimeSeries'),
 ('pandas.sparse.series', 'SparseTimeSeries'),
 ('pandas.tslib', 'NaT'),
 ('pandas.tslib', 'Timestamp')]

@dengemann
Copy link
Contributor Author

Btw. is there a 101 around on how to activate Travis-CI for ones fork?

@jreback
Copy link
Contributor

jreback commented Apr 13, 2013

@ghost
Copy link

ghost commented Apr 13, 2013

NaT and DateRange don't really need a docstring, the rest do.
Marking this as 0.11, since there's still a few days until 0.11 final,
and I'll put the missing docstrings together till then.

Thanks.

@ghost
Copy link

ghost commented Apr 14, 2013

Pushed directly to master with some added docstrings.

I believe this is your first commit to pandas. good job! 👍

@ghost ghost closed this Apr 14, 2013
@dengemann
Copy link
Contributor Author

Thanks @y-p ! ;-)

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants