-
Notifications
You must be signed in to change notification settings - Fork 62
change isinstance #792
change isinstance #792
Conversation
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.
Looks you need to change docstring according to the changes.
@@ -1995,16 +1995,16 @@ def pct_change_overload(df, periods=1, fill_method='pad', limit=None, freq=None) | |||
ty_checker = TypeChecker('Method {}().'.format(name)) | |||
ty_checker.check(df, DataFrameType) | |||
|
|||
if not isinstance(periods, (types.Integer, types.Omitted)): | |||
if not isinstance(periods, (types.Integer, types.Omitted)) and periods != 1: |
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.
Why periods
could not be equal to other values?
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.
@densmirn periods
could be equal to other values. This condition need that default value supported.
if not isinstance(limit, (types.Omitted, types.NoneType)) and limit is not None: | ||
ty_checker.raise_exc(limit, 'None', 'limit') | ||
|
||
if not isinstance(freq, (types.Omitted, types.NoneType)): | ||
if not isinstance(freq, (types.Omitted, types.NoneType)) and freq is not None: |
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.
Please add unit test on situation when limit
and freq
are equal to None
on the typing stage. Pay your attention that there are no such checks in Series.pct_change
overload.
@@ -1954,7 +1954,7 @@ def pct_change_overload(df, periods=1, fill_method='pad', limit=None, freq=None) | |||
|
|||
Limitations | |||
----------- | |||
Parameters ``limit`` and ``freq`` are unsupported. | |||
Parameters ``limit`` and ``freq`` are supported. |
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.
This statement doesn't look like a limitation.
No description provided.