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

Avoid circular import between core.series and plotting._xyz #16931

Closed
wants to merge 3 commits into from
Closed

Avoid circular import between core.series and plotting._xyz #16931

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6f60ff8
Avoid circular import between core.series and plotting._xyz
jbrockmendel Jul 14, 2017
c2840de
Check that isinstance(data, ABCSeries) == isinstance(data, Series)
jbrockmendel Jul 14, 2017
c9b78c1
Merge branch 'master' into nocircle
jbrockmendel Jul 15, 2017
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
4 changes: 2 additions & 2 deletions pandas/core/series.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@
from pandas._libs import index as libindex, tslib as libts, lib, iNaT
from pandas.core.config import get_option

import pandas.plotting._core as _gfx # noqa

__all__ = ['Series']

_shared_doc_kwargs = dict(
Expand Down Expand Up @@ -3060,8 +3062,6 @@ def create_from_value(value, index, dtype):
# ----------------------------------------------------------------------
# Add plotting methods to Series

import pandas.plotting._core as _gfx # noqa

Series.plot = base.AccessorProperty(_gfx.SeriesPlotMethods,
_gfx.SeriesPlotMethods)
Series.hist = _gfx.hist_series
Expand Down
16 changes: 14 additions & 2 deletions pandas/plotting/_core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@

from pandas.util._decorators import cache_readonly
from pandas.core.base import PandasObject

from pandas.core.dtypes.generic import ABCSeries
from pandas.core.dtypes.missing import notnull, remove_na_arraylike

from pandas.core.dtypes.common import (
is_list_like,
is_integer,
Expand All @@ -21,7 +24,7 @@
from pandas.core.common import AbstractMethodError, isnull, _try_sort
from pandas.core.generic import _shared_docs, _shared_doc_kwargs
from pandas.core.index import Index, MultiIndex
from pandas.core.series import Series

from pandas.core.indexes.period import PeriodIndex
from pandas.compat import range, lrange, map, zip, string_types
import pandas.compat as compat
Expand Down Expand Up @@ -334,7 +337,11 @@ def result(self):
def _compute_plot_data(self):
data = self.data

if isinstance(data, Series):
from pandas import Series
Copy link
Contributor

Choose a reason for hiding this comment

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

what is this about?

Copy link
Member Author

Choose a reason for hiding this comment

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

The first commit for this PR led to a bunch of TypeErrors on Travis that I can't reproduce locally. My best/only guess as to why is that the check isinstance(data, ABSeries) (now at L341) is somehow evaluating differently than the original isinstance(data, Series) (previously at L337).

So 337-339 is an attempt to check this guess. Regardless of whether the guess is correct, this check will be removed before long.

Copy link
Member Author

Choose a reason for hiding this comment

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

New hypothesis: Series._set_subtyp can sometimes set _subtyp to time_series, in which case it will not get recognized as ABCSeries. It tentatively looks like the Travis errors are date-related cases.

if isinstance(data, ABCSeries) != isinstance(data, Series):
raise Exception('WTFError', data)
Copy link
Member

Choose a reason for hiding this comment

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

We should make that error available only to developers when they rage-quit 😄


if isinstance(data, ABCSeries):
label = self.label
if label is None and data.name is None:
label = 'None'
Expand Down Expand Up @@ -1494,6 +1501,7 @@ def _args_adjust(self):

@classmethod
def _plot(cls, ax, y, column_num=None, return_type='axes', **kwds):
from pandas.core.series import remove_na
Copy link
Contributor

@jreback jreback Jul 14, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

I would be happy to move remove_na to pandas.core.dtypes.missing, and rename to remove_na_arraylike (will have to update a couple of references)

(pandas) bash-3.2$ grep -R remove_na pandas
pandas/core/series.py:            result = remove_na(self)
pandas/core/series.py:def remove_na(series):
pandas/plotting/_core.py:from pandas.core.series import Series, remove_na
pandas/plotting/_core.py:        y = remove_na(y)
pandas/plotting/_core.py:            y = [remove_na(v) for v in y]
pandas/plotting/_core.py:            y = remove_na(y)
pandas/plotting/_core.py:        values = [remove_na(v) for v in values]
pandas/tests/test_panel.py:from pandas.core.series import remove_na
pandas/tests/test_panel.py:                nona = remove_na(x)
pandas/tests/test_panel4d.py:from pandas.core.series import remove_na
pandas/tests/test_panel4d.py:                nona = remove_na(x)

Copy link
Contributor

Choose a reason for hiding this comment

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

this would allow the import to be at the top

Copy link
Member Author

Choose a reason for hiding this comment

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

That'd be great.

Copy link
Member Author

Choose a reason for hiding this comment

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

About to make a PR that makes this move and nothing else. We'll see if Travis still complains about that.

if y.ndim == 2:
y = [remove_na_arraylike(v) for v in y]
# Boxplot fails with empty arrays, so need to add a NaN
Expand Down Expand Up @@ -1566,6 +1574,7 @@ def maybe_color_bp(self, bp):

def _make_plot(self):
if self.subplots:
from pandas import Series
self._return_obj = Series()

for i, (label, y) in enumerate(self._iter_data()):
Expand Down Expand Up @@ -1968,6 +1977,7 @@ def maybe_color_bp(bp):
setp(bp['medians'], color=colors[2], alpha=1)

def plot_group(keys, values, ax):
from pandas.core.series import remove_na
keys = [pprint_thing(x) for x in keys]
values = [remove_na_arraylike(v) for v in values]
bp = ax.boxplot(values, **kwds)
Expand Down Expand Up @@ -2317,6 +2327,7 @@ def boxplot_frame_groupby(grouped, subplots=True, column=None, fontsize=None,
figsize=figsize, layout=layout)
axes = _flatten(axes)

from pandas import Series
ret = Series()
for (key, group), ax in zip(grouped, axes):
d = group.boxplot(ax=ax, column=column, fontsize=fontsize,
Expand Down Expand Up @@ -2388,6 +2399,7 @@ def _grouped_plot_by_column(plotf, data, columns=None, by=None,

_axes = _flatten(axes)

from pandas import Series
result = Series()
ax_values = []

Expand Down
3 changes: 1 addition & 2 deletions pandas/plotting/_tools.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

from pandas.core.dtypes.common import is_list_like
from pandas.core.index import Index
from pandas.core.series import Series
from pandas.compat import range


Expand Down Expand Up @@ -44,7 +43,7 @@ def table(ax, data, rowLabels=None, colLabels=None,
-------
matplotlib table object
"""
from pandas import DataFrame
from pandas import Series, DataFrame
if isinstance(data, Series):
data = DataFrame(data, columns=[data.name])
elif isinstance(data, DataFrame):
Expand Down