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

BUG: Timestamp constructor ignores unit parameter value #53198

Open
2 of 3 tasks
georgipeev opened this issue May 12, 2023 · 11 comments
Open
2 of 3 tasks

BUG: Timestamp constructor ignores unit parameter value #53198

georgipeev opened this issue May 12, 2023 · 11 comments
Assignees
Labels
Bug Non-Nano datetime64/timedelta64 with non-nanosecond resolution

Comments

@georgipeev
Copy link

georgipeev commented May 12, 2023

Pandas version checks

  • I have checked that this issue has not already been reported.

  • I have confirmed this bug exists on the latest version of pandas.

  • I have confirmed this bug exists on the main branch of pandas.

Reproducible Example

import pandas as pd
pd.Timestamp(2023, 5, 12, unit='ns').unit
# Returns 'us'
pd.Timestamp(2023, 5, 12, unit='s').unit
# Returns 'us'

Issue Description

It seems that in Pandas 2.0 the Timestamp constructor ignores the value of the unit parameter and always initializes the instance to unit='us'.
The unit of a Timestamp can, however, be changed correctly via the as_unit method.

Expected Behavior

>>> import pandas as pd
>>> pd.Timestamp(2023, 5, 12, unit='ns').unit
'ns'
>>> pd.Timestamp(2023, 5, 12, unit='s').unit
's'

Installed Versions

INSTALLED VERSIONS

commit : 37ea63d
python : 3.9.16.final.0
python-bits : 64
OS : Linux
OS-release : 4.18.0-348.23.1.el8_5.jump2.x86_64
Version : #1 SMP Tue Nov 1 14:32:55 CDT 2022
machine : x86_64
processor : x86_64
byteorder : little
LC_ALL : None
LANG : en_US.UTF-8
LOCALE : en_US.UTF-8

pandas : 2.0.1
numpy : 1.22.4
pytz : 2022.2.1
dateutil : 2.8.2
setuptools : 66.0.0
pip : 23.1.2
Cython : 0.29.32
pytest : 7.3.1
hypothesis : None
sphinx : 7.0.0
blosc : None
feather : None
xlsxwriter : None
lxml.etree : 4.9.1
html5lib : 1.1
pymysql : 1.0.2
psycopg2 : None
jinja2 : 3.1.2
IPython : None
pandas_datareader: None
bs4 : 4.11.2
bottleneck : None
brotli : None
fastparquet : None
fsspec : 2023.5.0
gcsfs : None
matplotlib : None
numba : 0.56.0
numexpr : None
odfpy : None
openpyxl : None
pandas_gbq : None
pyarrow : 12.0.0
pyreadstat : None
pyxlsb : None
s3fs : None
scipy : 1.10.1
snappy : None
sqlalchemy : 1.4.40
tables : None
tabulate : 0.8.10
xarray : None
xlrd : None
zstandard : 0.21.0
tzdata : 2022.2
qtpy : None
pyqt5 : None

@georgipeev georgipeev added Bug Needs Triage Issue that has not been reviewed by a pandas team member labels May 12, 2023
@alexprincel
Copy link
Contributor

I tracked this to this section of timestamps.pyx:

        elif is_integer_object(year):
            # User passed positional arguments:
            # Timestamp(year, month, day[, hour[, minute[, second[,
            # microsecond[, tzinfo]]]]])
            ts_input = datetime(ts_input, year, month, day or 0,
                                hour or 0, minute or 0, second or 0, fold=fold or 0)
            unit = None

We can see that unit is explicitly set to None, presumably because we are using datetime which does not have precision greater than seconds.

This is then passed to get_supported_reso. This would indicate that the return is ns but we observe us.

cpdef NPY_DATETIMEUNIT get_supported_reso(NPY_DATETIMEUNIT reso):
    # If we have an unsupported reso, return the nearest supported reso.
    if reso == NPY_DATETIMEUNIT.NPY_FR_GENERIC:
        # TODO: or raise ValueError? trying this gives unraisable errors, but
        #  "except? -1" breaks at compile-time for unknown reasons
        return NPY_DATETIMEUNIT.NPY_FR_ns
    if reso < NPY_DATETIMEUNIT.NPY_FR_s:
        return NPY_DATETIMEUNIT.NPY_FR_s
    elif reso > NPY_DATETIMEUNIT.NPY_FR_ns:
        return NPY_DATETIMEUNIT.NPY_FR_ns
    return reso

I may be looking in the wrong direction but hoping this helps others pinpoint more accurately.

@alexprincel
Copy link
Contributor

This is interesting, this behavior does not happen happen when passing the date components via keyword instead of by position. Will continue down that track:

>>> pd.Timestamp(2023, 1, 2, 3, 4, 5, 6, unit='ns').unit
'us'
>>> pd.Timestamp(year=2023, month=1, day=2, hour=3, minute=4, second=5, nanosecond=6, unit='ns').unit
'ns'

@alexprincel
Copy link
Contributor

I think I found part of the issue, the following piece of code in convert_to_tsobject executes when calling as per your example:

cdef _TSObject convert_to_tsobject(object ts, tzinfo tz, str unit,
                                   bint dayfirst, bint yearfirst, int32_t nanos=0):
    # ...
    cdef:
        _TSObject obj
        NPY_DATETIMEUNIT reso

    # ...
    if ts is None or ts is NaT:
        obj.value = NPY_NAT
    # ...
    elif PyDateTime_Check(ts):
        if nanos == 0:
            if isinstance(ts, ABCTimestamp):
                reso = abbrev_to_npy_unit(ts.unit)  # TODO: faster way to do this?
            else:
                # *********** This is what we hit that forces NPY_FR_us **********
                # TODO: what if user explicitly passes nanos=0?
                reso = NPY_FR_us
        else:
            reso = NPY_FR_ns
        return convert_datetime_to_tsobject(ts, tz, nanos, reso=reso)

Still need to figure out what is the resolution for this bug.

@alexprincel
Copy link
Contributor

take

@alexprincel
Copy link
Contributor

I think I better understand the situation now.

@georgipeev passing a year, month and date creates a python datetime under the hood, which has a microsecond ('us') precision. While it would be possible to force the unit keyword value to update the Timestamp object's unit property, it would not be truthful to how it was created.

If you specify nanoseconds when creating your timestamp it will have ns time precision by default. Does this help ?

@georgipeev
Copy link
Author

@alexprincel Thanks for investigating the underlying reason of this behavior.
I still believe that, in the end, implementation detail should not trump the promise the constructor's signature makes. I think there are two sensible ways to resolve this - either completely remove the unit parameter of the constructor, or add logic to override the default us unit that datetime uses (which is implementation detail).
Does that make sense to you?

@MarcoGorelli MarcoGorelli added Non-Nano datetime64/timedelta64 with non-nanosecond resolution and removed Needs Triage Issue that has not been reviewed by a pandas team member labels May 15, 2023
@alexprincel
Copy link
Contributor

@georgipeev I'll play around with substituting the python datetime for a np.datetime64 which has nanosecond precision and unenforce the precision limits.

@amgcc
Copy link

amgcc commented Mar 3, 2024

How is this still an issue almost a year later? If unit is not going to be read in the constructor, then it should be removed. Otherwise it should be evalutated.

Upgrading from pandas 2.0 to 2.1 has been quite a pain to deal with the timestamp inconsistencies.

@caseytomlin
Copy link

I was also bit by this.

Not sure if it's a separate issue/intended, but there is also a difference when passing a string vs. integer arguments:

>>> pd.__version__
'2.1.1'
>>> pd.Timestamp("2022-01-31", unit="ns").unit
's'
>>> pd.Timestamp("2022-01-31").unit
's'
>>> pd.Timestamp(year=2022, month=1, day=31).unit
'us'
>>> pd.Timestamp(year=2022, month=1, day=31, unit="ns").unit
'us'

@caseytomlin
Copy link

caseytomlin commented Mar 16, 2024

If you specify nanoseconds when creating your timestamp it will have ns time precision by default. Does this help ?

This seems to only be true if one specifies a non-zero nanosecond:

>>> pd.Timestamp(year=2022, month=1, day=31, nanosecond=0).unit
'us'
>>> pd.Timestamp(year=2022, month=1, day=31, nanosecond=1).unit
'ns'

but not always...

>>> t = pd.Timestamp("2022-01-31")
>>> t = t.replace(nanosecond=1)
>>> t
Timestamp('2022-01-31 00:00:00.000000001')
>>> t.unit
's'

@tuhinsharma121
Copy link
Contributor

Is anyone working on this? Can I work on it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Non-Nano datetime64/timedelta64 with non-nanosecond resolution
Projects
None yet
Development

No branches or pull requests

6 participants