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.

Sign up for GitHub

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

DOC: enable docstring on DataFrame.columns/index #20385

Merged
merged 4 commits into from
Mar 22, 2018
Merged

DOC: enable docstring on DataFrame.columns/index #20385

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6fcc2ef
DOC: enable docstring on DataFrame.columns/index
jorisvandenbossche Mar 16, 2018
10b1f3d
fix combine 2 cdefs
jorisvandenbossche Mar 19, 2018
8a38af6
Add actual summary lines
jorisvandenbossche Mar 20, 2018
c9a8a34
add to api.rst
jorisvandenbossche Mar 20, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions pandas/_libs/properties.pyx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,19 +42,22 @@ cache_readonly = CachedProperty


cdef class AxisProperty(object):
cdef:

cdef readonly:
Py_ssize_t axis
object __doc__
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this go under the same def readonly block as axis?

And do we want object or str for the type?

Copy link
Member Author

Choose a reason for hiding this comment

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

whoops, the above cdef readonly is a typo, I meant to keep it as readonly.
I am not fully sure if it is actually needed for axis, or if we can also make it readonly (I would think it is never modified internally, bot not sure)

For the object, it is what is done for CachedProperties above as well. My cython is not good enough to really know the consequences. I think we usually use object for strings in our cython code.

Copy link
Member Author

Choose a reason for hiding this comment

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

If the test pass with using readonly for axis, I will combine them in one cdef readonly

Copy link
Member

Choose a reason for hiding this comment

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

object vs str is pretty harmless in this case. I'd advocate sticking with object for now b/c then we don't need to worry about potential py2/py3 bytes/unicode corner cases.


def __init__(self, axis=0):
def __init__(self, axis=0, doc=""):
self.axis = axis
self.__doc__ = doc

def __get__(self, obj, type):
cdef:
list axes

if obj is None:
# Only instances have _data, not classes
return None
return self
else:
axes = obj._data.axes
return axes[self.axis]
Expand Down
2 changes: 1 addition & 1 deletion pandas/core/generic.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ def _setup_axes(cls, axes, info_axis=None, stat_axis=None, aliases=None,
if build_axes:

def set_axis(a, i):
setattr(cls, a, properties.AxisProperty(i))
setattr(cls, a, properties.AxisProperty(i, a))
Copy link
Member

Choose a reason for hiding this comment

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

This setattr has always bothered me. It kind of makes since in a general-case world with PanelND, but if we're only interested in Series/DataFrame/[barely-]Panel wouldn't it be clearer just to write:

class DataFrame(NDFrame):
    index = properties.AxisProperty(0)
    columns = properties.AxisProperty(1)

?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, fully agree that if we only have 1D/2D this makes much more sense. But maybe we should only clean that up when Panel is removed?

cls._internal_names_set.add(a)

if axes_are_reversed:
Expand Down
6 changes: 4 additions & 2 deletions pandas/tests/frame/test_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

# pylint: disable-msg=W0612,E1101
from copy import deepcopy
import pydoc
import sys
from distutils.version import LooseVersion

Expand Down Expand Up @@ -362,8 +363,9 @@ def test_axis_aliases(self):

def test_class_axis(self):
# https://github.com/pandas-dev/pandas/issues/18147
DataFrame.index # no exception!
DataFrame.columns # no exception!
# no exception and no empty docstring
assert pydoc.getdoc(DataFrame.index)
assert pydoc.getdoc(DataFrame.columns)

def test_more_values(self):
values = self.mixed_frame.values
Expand Down
4 changes: 3 additions & 1 deletion pandas/tests/series/test_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# coding=utf-8
# pylint: disable-msg=E1101,W0612
from collections import OrderedDict
import pydoc

import pytest

Expand Down Expand Up @@ -384,7 +385,8 @@ def test_axis_alias(self):

def test_class_axis(self):
# https://github.com/pandas-dev/pandas/issues/18147
Series.index # no exception!
# no exception and no empty docstring
assert pydoc.getdoc(Series.index)

def test_numpy_unique(self):
# it works!
Expand Down