cleaned up imports #18264

tdpetrou · 2017-11-13T17:47:04Z

This is a continuation of #18148 and does the following

puts the reshape imports from pandas.core.api to pandas.core.reshape.api
uses pandas.core.dtypes.generic.ABCMultiIndex inplace of MultiIndex in melt.py
uses from pandas import DataFrame in melt.py inside functions
imports directly from pandas in reshape tests

There is one test failing. Statsmodels imports get_dummies from pandas.core.api which is no longer possible. It should import directly from pandas.

jorisvandenbossche · 2017-11-13T21:17:41Z

@jreback Why is it needed to use ABCMultiIndex instead of MultiIndex and move the DataFrame imports inline? There is no circular import here?

jorisvandenbossche · 2017-11-13T21:18:38Z

There is one test failing. Statsmodels imports get_dummies from pandas.core.api which is no longer possible. It should import directly from pandas.

Then I think we should put it back, but we should also open an issue at statsmodels that they should not do that (independently of whether we have that import there or not, they should never import from 'core')

tdpetrou · 2017-11-13T21:32:37Z

@jorisvandenbossche Looks like statsmodels has it fixed now.

Edited:

@TomAugspurger fixed this here: statsmodels/statsmodels@45e55d0#diff-a494553e88130577065e0b012246ca51

jorisvandenbossche · 2017-11-13T21:42:54Z

The problem is that they didn't yet do a new release. So for now I would not do any change that breaks a released version of statsmodels, as we are not sure at this point a new statsmodel release will be out before a new pandas release.

tdpetrou · 2017-11-13T21:51:25Z

Yes, that makes sense. But, pandas 0.22 won't come out for another 3+ months (where is the release schedule btw?) and statsmodels has been on 0.8 for 9 months now. Should we find out if they are going to release the next version before 0.22?

TomAugspurger · 2017-11-13T22:00:36Z

Are you able to just import get_dummies in pandas.core.api, or does that cause circular import issues? I wouldn't assume that statsmodels is going to do a release before 0.22.

…

On Mon, Nov 13, 2017 at 3:53 PM, Ted Petrou ***@***.***> wrote: Yes, that makes sense. But, pandas 0.22 won't come out for another 3+ months (where is the release schedule btw?) and statsmodels has been on 0.8 for 9 months now. Should we find out if they are going to release the next version before 0.22? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#18264 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABQHIo3Lqo9WK81S6WqD38DqkHHe1HaSks5s2LpqgaJpZM4QcJx7> .

codecov · 2017-11-13T23:20:52Z

Codecov Report

Merging #18264 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #18264      +/-   ##
==========================================
+ Coverage   91.38%   91.38%   +<.01%     
==========================================
  Files         164      164              
  Lines       49884    49884              
==========================================
+ Hits        45584    45587       +3     
+ Misses       4300     4297       -3

Flag	Coverage Δ
#multiple	`89.19% <100%> (+0.02%)`	⬆️
#single	`39.41% <33.33%> (-0.07%)`	⬇️

Impacted Files	Coverage Δ
pandas/core/api.py	`100% <ø> (ø)`	⬆️
pandas/core/reshape/melt.py	`96.22% <100%> (+0.03%)`	⬆️
pandas/core/reshape/api.py	`100% <100%> (ø)`	⬆️
pandas/io/gbq.py	`25% <0%> (-58.34%)`	⬇️
pandas/core/frame.py	`97.8% <0%> (-0.1%)`	⬇️
pandas/plotting/_converter.py	`65.25% <0%> (+1.81%)`	⬆️

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 7f4c960...8782852. Read the comment docs.

jorisvandenbossche · 2017-11-13T23:21:50Z

Are you able to just import get_dummies in pandas.core.api, or does that cause circular import issues?

That is no problem, as we were only cleaning up imports in core/api.py that are not really necessary from pandas point of view (but thus used by statsmodels). So I mean easy to just not do the clean-up (yet)

jreback · 2017-11-14T11:17:18Z

@tdpetrou

There is one test failing. Statsmodels imports get_dummies from pandas.core.api which is no longer possible. It should import directly from pandas.

so test_downstream.py is failing? we test against the latest released versions of these packages

I want to add a repro test with the statsmodels failure, then you can leave get_dummies import in pandas.core.api, with a TODO to move later (and we'll create an issue about it).

TomAugspurger · 2017-11-14T11:32:39Z

It's been fixed in statsmodels master, just not the released version.

…

On Tue, Nov 14, 2017 at 5:17 AM, Jeff Reback ***@***.***> wrote: @tdpetrou <https://github.com/tdpetrou> There is one test failing. Statsmodels imports get_dummies from pandas.core.api which is no longer possible. It should import directly from pandas. so test_downstream.py is failing? I want to add a repro test with the statsmodels failure, then you can leave get_dummies import in pandas.core.api, with a TODO to move later (and we'll create an issue about it). — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#18264 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABQHIjsact-7pBl3mn2OPd30Z1I-IUhQks5s2XbDgaJpZM4QcJx7> .

jreback · 2017-11-14T11:35:27Z

@TomAugspurger that's good, but our test should still fail; its against the released version.

if its NOT failing, then our test is not good enough (maybe it doesn't import that code).

tdpetrou · 2017-11-14T19:49:38Z

@jreback Yes it was test_downstream.py. Ok I'll keep it get_dummies import in pandas.core.api with TODO

pandas/tests/test_downstream.py:60:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
../../anaconda3/lib/python3.6/site-packages/statsmodels/api.py:14: in <module>
    from .discrete.discrete_model import (Poisson, Logit, Probit,
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

    """
    from __future__ import division

    __all__ = ["Poisson", "Logit", "Probit", "MNLogit", "NegativeBinomial"]

    from statsmodels.compat.python import lmap, lzip, range
    import numpy as np
    from scipy.special import gammaln
    from scipy import stats, special, optimize  # opt just for nbin
    import statsmodels.tools.tools as tools
    from statsmodels.tools import data as data_tools
    from statsmodels.tools.decorators import (resettable_cache,
            cache_readonly)
    from statsmodels.regression.linear_model import OLS
    from scipy import stats, special, optimize  # opt just for nbin
    from scipy.stats import nbinom
    from statsmodels.tools.sm_exceptions import PerfectSeparationError
    from statsmodels.tools.numdiff import (approx_fprime, approx_hess,
                                           approx_hess_cs, approx_fprime_cs)
    import statsmodels.base.model as base
    from statsmodels.base.data import handle_data  # for mnlogit
    import statsmodels.regression.linear_model as lm
    import statsmodels.base.wrapper as wrap
    from statsmodels.compat.numpy import np_matrix_rank
>   from pandas.core.api import get_dummies
E   ImportError: cannot import name 'get_dummies'

../../anaconda3/lib/python3.6/site-packages/statsmodels/discrete/discrete_model.py:41: ImportError

jreback · 2017-11-15T11:38:11Z

@tdpetrou so this PR should be failing as the current code has the move of get_dummies. yes I want to leave it until its fixed in statsmodels, but why does this NOT fail? this would indicate the test is flawed.

tdpetrou · 2017-11-16T15:46:34Z

@jreback test_downstream.py is failing on my machine. Are you implying that the CI services are linking to the unreleased statsmodels versions?

jreback · 2017-11-17T00:53:22Z

this is puzzling why this is not failing, going to test on another build, #18333 let's see.

jreback · 2017-11-17T11:30:43Z

@tdpetrou go ahead and rebase, this should fail.

codecov · 2017-11-17T19:59:45Z

Codecov Report

Merging #18264 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #18264      +/-   ##
==========================================
+ Coverage   91.38%   91.38%   +<.01%     
==========================================
  Files         164      164              
  Lines       49790    49884      +94     
==========================================
+ Hits        45501    45587      +86     
- Misses       4289     4297       +8

Flag	Coverage Δ
#multiple	`89.19% <100%> (+0.02%)`	⬆️
#single	`39.41% <33.33%> (-0.15%)`	⬇️

Impacted Files	Coverage Δ
pandas/core/api.py	`100% <ø> (ø)`	⬆️
pandas/core/reshape/melt.py	`96.22% <100%> (+0.03%)`	⬆️
pandas/core/reshape/api.py	`100% <100%> (ø)`	⬆️
pandas/io/gbq.py	`25% <0%> (-58.34%)`	⬇️
pandas/util/_decorators.py	`80.7% <0%> (-0.34%)`	⬇️
pandas/core/frame.py	`97.8% <0%> (-0.1%)`	⬇️
pandas/core/sparse/series.py	`95.28% <0%> (-0.06%)`	⬇️
pandas/core/series.py	`95% <0%> (-0.02%)`	⬇️
pandas/core/generic.py	`95.72% <0%> (-0.01%)`	⬇️
pandas/core/reshape/reshape.py	`100% <0%> (ø)`	⬆️
... and 15 more

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 cfad581...794dd6d. Read the comment docs.

codecov · 2017-11-17T19:59:56Z

Codecov Report

Merging #18264 into master will increase coverage by 0.29%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #18264      +/-   ##
==========================================
+ Coverage   91.35%   91.65%   +0.29%     
==========================================
  Files         163      164       +1     
  Lines       49681    51487    +1806     
==========================================
+ Hits        45386    47189    +1803     
- Misses       4295     4298       +3

Flag	Coverage Δ
#multiple	`89.52% <100%> (+0.39%)`	⬆️
#single	`40.54% <33.33%> (+0.8%)`	⬆️

Impacted Files	Coverage Δ
pandas/core/api.py	`100% <ø> (ø)`	⬆️
pandas/core/reshape/melt.py	`96.22% <100%> (+0.03%)`	⬆️
pandas/core/reshape/api.py	`100% <100%> (ø)`	⬆️
pandas/io/gbq.py	`25% <0%> (-58.34%)`	⬇️
pandas/tseries/offsets.py	`96.7% <0%> (-0.31%)`	⬇️
pandas/core/resample.py	`96.16% <0%> (-0.19%)`	⬇️
pandas/core/frame.py	`97.8% <0%> (-0.1%)`	⬇️
pandas/compat/__init__.py	`58.1% <0%> (-0.09%)`	⬇️
pandas/core/indexes/timedeltas.py	`91.14% <0%> (-0.04%)`	⬇️
pandas/core/generic.py	`95.73% <0%> (ø)`	⬆️
... and 9 more

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 dac9b43...1f56dfd. Read the comment docs.

jreback · 2017-11-19T15:06:30Z

can you rebase 1 more time, I want to see this fail; the statsmodels test was being skipped.

tdpetrou · 2017-11-22T14:58:03Z

@jreback Did this fail correctly?

jorisvandenbossche · 2017-11-22T22:05:29Z

Yes, it is correctly failing now. So you can now put it back in core/api.py

@jreback can you answer my question above #18264 (comment) ?

tdpetrou · 2017-11-22T22:24:57Z

@jorisvandenbossche I put get_dummies back in core.api. I left the get_dummies import in core.reshape.api.

jreback · 2017-11-22T22:33:51Z

Why is it needed to use ABCMultiIndex instead of MultiIndex and move the DataFrame imports inline? There is no circular import here?

we use the ABC* for instance checking generally, these have no dependencies. This actually does have a conditional import, that's why its pandas.core.frame.DataFrame; we try to remove these and import inside the function as its much cleaner.

jreback · 2017-11-22T22:34:36Z

@tdpetrou

I put get_dummies back in core.api. I left the get_dummies import in core.reshape.api.

thanks. ping on green.

Can you make an issue to move this (on the pandas side, when statsmodels releases).

tdpetrou · 2017-11-23T15:46:48Z

@jreback I think this is good but circleci is failing?

I'll keep a note to make an issue on the next statsmodels release

jreback · 2017-11-23T15:51:07Z

thanks @tdpetrou

I'll keep a note to make an issue on the next statsmodels release

can you make an issue about this, we have 2 changes we need to make now (I'll edit with the other one)

jorisvandenbossche added Clean Reshaping Concat, Merge/Join, Stack/Unstack, Explode labels Nov 13, 2017

pandas-dev deleted a comment from codecov bot Nov 13, 2017

tdpetrou added 2 commits November 22, 2017 17:14

cleaned up imports

eb70bcb

put get_dummies back in core.api

1f56dfd

jreback added this to the 0.22.0 milestone Nov 23, 2017

jreback merged commit 04b628f into pandas-dev:master Nov 23, 2017

tdpetrou deleted the simultaneous-melt branch November 23, 2017 16:19

tdpetrou mentioned this pull request Nov 23, 2017

COMPAT: statsmodel 0.9.0 #18452

Closed

jbrockmendel mentioned this pull request Nov 24, 2019

DEPR: remove statsmodels/seaborn compat shims #29822

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cleaned up imports #18264

cleaned up imports #18264

tdpetrou commented Nov 13, 2017

jorisvandenbossche commented Nov 13, 2017

jorisvandenbossche commented Nov 13, 2017

tdpetrou commented Nov 13, 2017 •

edited

Loading

jorisvandenbossche commented Nov 13, 2017 •

edited

Loading

tdpetrou commented Nov 13, 2017

TomAugspurger commented Nov 13, 2017 via email

codecov bot commented Nov 13, 2017

jorisvandenbossche commented Nov 13, 2017

jreback commented Nov 14, 2017 •

edited

Loading

TomAugspurger commented Nov 14, 2017 via email

jreback commented Nov 14, 2017 •

edited

Loading

tdpetrou commented Nov 14, 2017

jreback commented Nov 15, 2017

tdpetrou commented Nov 16, 2017

jreback commented Nov 17, 2017

jreback commented Nov 17, 2017

codecov bot commented Nov 17, 2017

codecov bot commented Nov 17, 2017 •

edited

Loading

jreback commented Nov 19, 2017

tdpetrou commented Nov 22, 2017

jorisvandenbossche commented Nov 22, 2017

tdpetrou commented Nov 22, 2017

jreback commented Nov 22, 2017

jreback commented Nov 22, 2017

tdpetrou commented Nov 23, 2017

jreback commented Nov 23, 2017

cleaned up imports #18264

cleaned up imports #18264

Conversation

tdpetrou commented Nov 13, 2017

jorisvandenbossche commented Nov 13, 2017

jorisvandenbossche commented Nov 13, 2017

tdpetrou commented Nov 13, 2017 • edited Loading

jorisvandenbossche commented Nov 13, 2017 • edited Loading

tdpetrou commented Nov 13, 2017

TomAugspurger commented Nov 13, 2017 via email

codecov bot commented Nov 13, 2017

Codecov Report

jorisvandenbossche commented Nov 13, 2017

jreback commented Nov 14, 2017 • edited Loading

TomAugspurger commented Nov 14, 2017 via email

jreback commented Nov 14, 2017 • edited Loading

tdpetrou commented Nov 14, 2017

jreback commented Nov 15, 2017

tdpetrou commented Nov 16, 2017

jreback commented Nov 17, 2017

jreback commented Nov 17, 2017

codecov bot commented Nov 17, 2017

Codecov Report

codecov bot commented Nov 17, 2017 • edited Loading

Codecov Report

jreback commented Nov 19, 2017

tdpetrou commented Nov 22, 2017

jorisvandenbossche commented Nov 22, 2017

tdpetrou commented Nov 22, 2017

jreback commented Nov 22, 2017

jreback commented Nov 22, 2017

tdpetrou commented Nov 23, 2017

jreback commented Nov 23, 2017

tdpetrou commented Nov 13, 2017 •

edited

Loading

jorisvandenbossche commented Nov 13, 2017 •

edited

Loading

jreback commented Nov 14, 2017 •

edited

Loading

jreback commented Nov 14, 2017 •

edited

Loading

codecov bot commented Nov 17, 2017 •

edited

Loading