Skip to content
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

pd.NaT.date() returns datetime.date(1, 255, 255) #9513

Closed
s-celles opened this issue Feb 18, 2015 · 12 comments
Closed

pd.NaT.date() returns datetime.date(1, 255, 255) #9513

s-celles opened this issue Feb 18, 2015 · 12 comments
Labels
Datetime Datetime data dtype Error Reporting Incorrect or improved errors from pandas
Milestone

Comments

@s-celles
Copy link
Contributor

Hello,

pd.NaT.date() returns datetime.date(1, 255, 255)
that's an odd results
I was expecting None (or something else) because that's really strange to have
date of year "1" in an Excel file

Kind regards

@shoyer
Copy link
Member

shoyer commented Feb 18, 2015

Could we make NaT.date() just raise a ValueError? I think I would prefer that over None, which would usually just mean differing the error until later.

@s-celles
Copy link
Contributor Author

Yes raising a ValueError is more understandable.

see http://stackoverflow.com/questions/18212247/convert-text-dates-to-dates-and-then-keeping-the-na-values

@shoyer
Copy link
Member

shoyer commented Feb 19, 2015

If you'd like to put together a PR to fix this, that would be great!

@s-celles
Copy link
Contributor Author

Yes but where is defined Not A Time.
What kind of behaviour should we have ? Does getting any attribute / applying any method should raise an error ?

@shoyer
Copy link
Member

shoyer commented Feb 19, 2015

NaT is defined by the NaTType class here: https://github.com/pydata/pandas/blob/bceb342d76ca4061fc8d3a1089bc4e83a32da656/pandas/tslib.pyx#L568

NaTType is an indirect subclass of datetime, so it inherits the default date method. You simply need to supply the method on NaTType, e.g.,

def date(self):
    raise ValueError('some helpful message')

(and yes, this is Cython, but you can write pure Python code here.)

This will also need a test to verify that the error is raised, probably somewhere around here:
https://github.com/pydata/pandas/blob/bceb342d76ca4061fc8d3a1089bc4e83a32da656/pandas/tseries/tests/test_tslib.py#L589

@shoyer
Copy link
Member

shoyer commented Feb 19, 2015

If there are other methods that are not returning appropriate values, feel free to override them to raise an error as well. But most of them should be supplying appropriate returns values already (e.g., nan for NaT.year).

@s-celles
Copy link
Contributor Author

I directly edited code on GitHub and send PR this way (because I don't have a Python install here)... I just hope that Travis CI will be ok.

@s-celles
Copy link
Contributor Author

Sorry but it seems that my PR fails with CI but I try locally it should works. Any idea ?

@s-celles
Copy link
Contributor Author

An other PR #9523

@jreback jreback added Datetime Datetime data dtype API Design Error Reporting Incorrect or improved errors from pandas and removed API Design labels Feb 20, 2015
@kelvin22
Copy link

Probably related:

When writing the NaT to postgres using SQLAlchemy:

sqlalchemy.exc.DataError: (psycopg2.DataError) invalid input syntax for type timestamp: "0001-255-255T00:00:00"

Although I thought this was fixed a while back
#6701
#2754

Note:
pandas==0.16.1
psycopg2==2.6
SQLAlchemy==1.0.4

@jorisvandenbossche
Copy link
Member

@kelvin22 Can you open a separate issue for this (because as you say, this should be fixed). If possible, please provide a reproducible example.

@jreback jreback added this to the Next Major Release milestone May 20, 2015
@jreback jreback modified the milestones: 0.17.0, Next Major Release Jun 17, 2015
@kawochen
Copy link
Contributor

should be closed too?

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
Development

Successfully merging a pull request may close this issue.

6 participants