-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
Timestamp.replace raises ValueError instead of TypeError when given wrong argument #15240
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
Comments
if you want to do a PR wouldn be great |
replace **kwd argument to explicit argument to raise TypeError when wrong argument name is given
I am having a try :-) |
I can't really test my patch on my machine (as I have not the tool suite necessary to compile pandas). Can I start a PR (even unfinished) to test via the Travis/Appveyor tests ? or is it "bad practice" ? |
@sdementen the tests are exactly in the above link, IOW in pandas/tseries/test_timezones.py I would recommend you install a proper dev version of pandas to test: http://pandas.pydata.org/pandas-docs/stable/contributing.html |
@jreback I can't find any test of the Timestamp.replace method (except tests testing the replace(tzinfo=xxx)). Are there specific tests for Timestamp.replace ? I was expecting/looking for tests like
|
you could add some in |
replace **kwd argument to explicit argument to raise TypeError when wrong argument name is given
Hi All, I've been looking to contribute to pandas for a while, so I've made a PR that I think closes this. followed the guide as best I could. |
replace **kwd argument to explicit argument to raise TypeError when wrong argument name is given
closes pandas-dev#15240 Author: Sébastien de Menten <[email protected]> Author: sdementen <[email protected]> Author: Jeff Reback <[email protected]> Closes pandas-dev#15248 from sdementen/timestamp-replace-compatibility and squashes the following commits: e70e75a [Jeff Reback] use tzinfo=object to accept None as a default parameter 52b6c2c [Sébastien de Menten] improve description of whatsnew item 9455ee9 [sdementen] fix for issue pandas-dev#15240 1a40777 [Sébastien de Menten] update what's new with bug fix 5263054 [Sébastien de Menten] fix typos + update doc d5c7b0a [Sébastien de Menten] update exception type raised when wrong argument to Timestamp.replace 2ac141a [sdementen] fix for issue pandas-dev#15240
Code Sample, a copy-pastable example if possible
Problem description
The reimplementation of Timestamp.replace (f8bd08e#diff-05de2950b2c86c9828531957e02d1b87L662) raises a ValueError when a keyword argument does not exist.
Previous implementation used datetime.replace that raises a TypeError when an argument is not valid.
This is a regression and it is probably better to keep API compatibility with the datetime.replace function (modulo extra functionalities).
For autodocumentation/autocomplete purposes, it could also be useful to have explicitly in the function signature the keywords, so instead of::
def replace(self, **kwds):
have::
def replace(self, year=None, month=None, day=None, hour=None, minute=None, second=None, microsecond=None, nanosecond=None, tzinfo=None)
Actual Output
Traceback (most recent call last):
File "", line 1, in
File "pandas\tslib.pyx", line 715, in pandas.tslib.Timestamp.replace (pandas\tslib.c:14832)
ValueError: invalid name apple passed
Expected Output
Traceback (most recent call last):
File "", line 1, in
File "pandas\tslib.pyx", line 715, in pandas.tslib.Timestamp.replace (pandas\tslib.c:14832)
TypeError: 'apple' is an invalid keyword argument for this function
Output of
pd.show_versions()
pandas: 0.19.2
nose: None
pip: 9.0.1
setuptools: 27.2.0
Cython: None
numpy: 1.11.3
scipy: None
statsmodels: None
xarray: None
IPython: 5.1.0
sphinx: 1.5.2
patsy: None
dateutil: 2.6.0
pytz: 2016.10
blosc: None
bottleneck: None
tables: None
numexpr: None
matplotlib: None
openpyxl: None
xlrd: None
xlwt: None
xlsxwriter: None
lxml: None
bs4: None
html5lib: None
httplib2: None
apiclient: None
sqlalchemy: None
pymysql: None
psycopg2: None
jinja2: 2.9.4
boto: None
pandas_datareader: None
The text was updated successfully, but these errors were encountered: