-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
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
Support for OO Optimization #21093
Changes from all commits
840eec8
19e71e3
95826d3
0e48c18
2248137
abdc5f2
e4ba948
021786b
a1b409d
a89c4f4
078610a
91a2226
c697dae
d6c66ec
c853c67
c825636
f8ccfa0
e427f4b
46a2f6a
a2e748a
75dbaad
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1090,12 +1090,17 @@ def apply(self, other): | |
|
||
|
||
class CustomBusinessMonthEnd(_CustomBusinessMonth): | ||
__doc__ = _CustomBusinessMonth.__doc__.replace('[BEGIN/END]', 'end') | ||
# TODO(py27): Replace condition with Subsitution after dropping Py27 | ||
if _CustomBusinessMonth.__doc__: | ||
__doc__ = _CustomBusinessMonth.__doc__.replace('[BEGIN/END]', 'end') | ||
_prefix = 'CBM' | ||
|
||
|
||
class CustomBusinessMonthBegin(_CustomBusinessMonth): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think better to do this with There was a problem hiding this comment. Choose a reason for hiding this commentThe 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') | ||
# TODO(py27): Replace condition with Subsitution after dropping Py27 | ||
if _CustomBusinessMonth.__doc__: | ||
__doc__ = _CustomBusinessMonth.__doc__.replace('[BEGIN/END]', | ||
'beginning') | ||
_prefix = 'CBMS' | ||
|
||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,7 +4,7 @@ | |
import types | ||
import warnings | ||
from textwrap import dedent, wrap | ||
from functools import wraps, update_wrapper | ||
from functools import wraps, update_wrapper, WRAPPER_ASSIGNMENTS | ||
|
||
|
||
def deprecate(name, alternative, version, alt_name=None, | ||
|
@@ -20,44 +20,45 @@ 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 of 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) | ||
|
||
@wraps(alternative) | ||
# adding deprecated directive to the docstring | ||
msg = msg or 'Use `{alt_name}` instead.'.format(alt_name=alt_name) | ||
msg = '\n '.join(wrap(msg, 70)) | ||
|
||
@Substitution(version=version, msg=msg) | ||
@Appender(alternative.__doc__) | ||
def wrapper(*args, **kwargs): | ||
""" | ||
.. deprecated:: %(version)s | ||
|
||
%(msg)s | ||
|
||
""" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
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 | ||
# Since we are using Substitution to create the required docstring, | ||
# remove that from the attributes that should be assigned to the wrapper | ||
assignments = tuple(x for x in WRAPPER_ASSIGNMENTS if x != '__doc__') | ||
update_wrapper(wrapper, alternative, assigned=assignments) | ||
|
||
return wrapper | ||
|
||
|
There was a problem hiding this comment.
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.There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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)