Skip to content

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

Closed
sdementen opened this issue Jan 27, 2017 · 7 comments
Labels
Datetime Datetime data dtype Error Reporting Incorrect or improved errors from pandas
Milestone

Comments

@sdementen
Copy link
Contributor

Code Sample, a copy-pastable example if possible

from pandas import Timestamp
Timestamp("2012").replace(apple=3)

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()

INSTALLED VERSIONS ------------------ commit: None python: 3.5.2.final.0 python-bits: 64 OS: Windows OS-release: 7 machine: AMD64 processor: Intel64 Family 6 Model 61 Stepping 4, GenuineIntel byteorder: little LC_ALL: None LANG: None LOCALE: None.None

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

@jreback
Copy link
Contributor

jreback commented Jan 27, 2017

if you want to do a PR wouldn be great

@jreback jreback added Difficulty Novice Error Reporting Incorrect or improved errors from pandas Datetime Datetime data dtype labels Jan 27, 2017
@jreback jreback added this to the 0.20.0 milestone Jan 27, 2017
sdementen added a commit to sdementen/pandas that referenced this issue Jan 27, 2017
replace **kwd argument to explicit argument to raise TypeError when wrong argument name is given
@sdementen
Copy link
Contributor Author

I am having a try :-)
Could you tell me where I can add an additional test on Timestamp.replace ? I have a hard time locating the tests...

@sdementen
Copy link
Contributor Author

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" ?

@jreback
Copy link
Contributor

jreback commented Jan 27, 2017

@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

@sdementen
Copy link
Contributor Author

@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

dt = pandas.Timestamp("2016")
tm.assert(dt.hour,0)
tm.assert(dt.replace(hour=3).hour,3)

@jreback
Copy link
Contributor

jreback commented Jan 27, 2017

https://github.com/pandas-dev/pandas/blob/master/pandas/tseries/tests/test_timezones.py#L1170

you could add some in test_timeseries.py if you want (and just leave these for the actual timezone replacement), but doesn't really matter.

sdementen added a commit to sdementen/pandas that referenced this issue Jan 27, 2017
replace **kwd argument to explicit argument to raise TypeError when wrong argument name is given
@AlexTheProgrammer
Copy link

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.

jreback pushed a commit to sdementen/pandas that referenced this issue Jan 28, 2017
replace **kwd argument to explicit argument to raise TypeError when wrong argument name is given
AnkurDedania pushed a commit to AnkurDedania/pandas that referenced this issue Mar 21, 2017
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Datetime Datetime data dtype Error Reporting Incorrect or improved errors from pandas
Projects
None yet
3 participants