Skip to content

Support for OO Optimization #21093

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

Merged
merged 21 commits into from
May 30, 2018
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
7 changes: 7 additions & 0 deletions pandas/tests/test_downstream.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
"""
Testing that we work in the downstream packages
"""
import subprocess

import pytest
import numpy as np # noqa
from pandas import DataFrame
Expand Down Expand Up @@ -53,6 +55,11 @@ def test_xarray(df):
assert df.to_xarray() is not None


def test_oo_optimizable():
# GH 21071
subprocess.check_call(["python", "-OO", "-c", "import pandas"])
Copy link

Choose a reason for hiding this comment

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

Does pandas import all modules at import pandas? If not, maybe this will be helpful.

Copy link
Member

Choose a reason for hiding this comment

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

@WillAyd do we want to go this way to also check some submodules?

Copy link
Member Author

Choose a reason for hiding this comment

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

I can take a look - if it's not a lot of effort I suppose it doesn't hurt

Copy link
Member Author

Choose a reason for hiding this comment

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

I've been messing around with this for a bit but AFAICT it keeps getting choked up when trying to import modules containing optional dependencies (ex: pandas.io.s3). I can't think of a simple way to configure the test so that it can differentiate intentional ImportErrors as a result of missing dependencies.

For now I'm going to revert to just working with the top level import - can open a separate issue if we wanted this to work recursively

Copy link
Member

Choose a reason for hiding this comment

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

Yep, if it turns out difficult, on problem in starting with the basic test (which already tests the large majority of our API)



@tm.network
def test_statsmodels():

Expand Down
10 changes: 6 additions & 4 deletions pandas/tseries/offsets.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
# import after tools, dateutil check
from dateutil.easter import easter
from pandas._libs import tslib, Timestamp, OutOfBoundsDatetime, Timedelta
from pandas.util._decorators import cache_readonly
from pandas.util._decorators import cache_readonly, Appender, Substitution

from pandas._libs.tslibs import ccalendar, frequencies as libfrequencies
from pandas._libs.tslibs.timedeltas import delta_to_nanoseconds
Expand Down Expand Up @@ -1011,7 +1011,7 @@ class BusinessMonthBegin(MonthOffset):
class _CustomBusinessMonth(_CustomMixin, BusinessMixin, MonthOffset):
"""
DateOffset subclass representing one custom business month, incrementing
between [BEGIN/END] of month dates
between %(increment)s of month dates

Parameters
----------
Expand Down Expand Up @@ -1089,13 +1089,15 @@ def apply(self, other):
return result


@Substitution(increment='end')
@Appender(_CustomBusinessMonth.__doc__)
class CustomBusinessMonthEnd(_CustomBusinessMonth):
__doc__ = _CustomBusinessMonth.__doc__.replace('[BEGIN/END]', 'end')
_prefix = 'CBM'


@Substitution(increment='beginning')
@Appender(_CustomBusinessMonth.__doc__)
class CustomBusinessMonthBegin(_CustomBusinessMonth):
Copy link
Contributor

Choose a reason for hiding this comment

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

I think better to do this with @subsitution and template this. We dont' use this pattern anywhere and might be confusing.

Copy link
Member Author

Choose a reason for hiding this comment

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

Makes sense - I'll take a look and repush

__doc__ = _CustomBusinessMonth.__doc__.replace('[BEGIN/END]', 'beginning')
_prefix = 'CBMS'


Expand Down
44 changes: 20 additions & 24 deletions pandas/util/_decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import inspect
import types
import warnings
from textwrap import dedent, wrap
from textwrap import dedent
from functools import wraps, update_wrapper


Expand All @@ -19,46 +19,42 @@ def deprecate(name, alternative, version, alt_name=None,

Parameters
----------
name : str
Name of function to deprecate
alternative : str
Name of function to use instead
name : func
Copy link
Member Author

Choose a reason for hiding this comment

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

Unless I am overlooking something I didn't see where this decorator was being used, but it seems like there is a mismatch between what was documented and the types it actually accepts (at least looking at alternative).

To make this work with substitution I took the liberty of assuming these arguments should functions and not strings whenever this decorator gets used

Copy link
Member

Choose a reason for hiding this comment

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

Unless I am overlooking something I didn't see where this decorator was being used,

It's not used a lot, but at least it is used for argmin/argmax and for scatter_matrix (it is not used there as a decorator, so I searched for "deprecate("

Function to deprecate.
alternative : func
Function to use instead.
version : str
Version of pandas in which the method has been deprecated
Version of pandas in which the method has been deprecated.
alt_name : str, optional
Name to use in preference of alternative.__name__
Name to use in preference of alternative.__name__.
klass : Warning, default FutureWarning
stacklevel : int, default 2
msg : str
The message to display in the warning.
Default is '{name} is deprecated. Use {alt_name} instead.'
The message to display in the warning.
Default is '{name} is deprecated. Use {alt_name} instead.'
"""

alt_name = alt_name or alternative.__name__
klass = klass or FutureWarning
warning_msg = msg or '{} is deprecated, use {} instead'.format(name,
alt_name)

# adding deprecated directive to the docstring
msg = msg or 'Use `{alt_name}` instead.'.format(alt_name=alt_name)

@Substitution(version=version, msg=msg)
@Appender(name.__doc__)
@wraps(alternative)
def wrapper(*args, **kwargs):
"""
.. deprecated:: %(version)s

%(msg)s

"""
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure why, but it seems this does not give the correct docstring.
Can you check that pd.Series.argmin? still gives the deprecation warning at the top? (I checked out this branch and didn't see it, while on master it is there)

warnings.warn(warning_msg, klass, stacklevel=stacklevel)
return alternative(*args, **kwargs)

# adding deprecated directive to the docstring
msg = msg or 'Use `{alt_name}` instead.'.format(alt_name=alt_name)
tpl = dedent("""
.. deprecated:: {version}

{msg}

{rest}
""")
rest = getattr(wrapper, '__doc__', '')
docstring = tpl.format(version=version,
msg='\n '.join(wrap(msg, 70)),
rest=dedent(rest))
wrapper.__doc__ = docstring

return wrapper


Expand Down