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: Fixed ignoring of nanoseconds when adding to series #47856 #48008

Merged
merged 8 commits into from
Aug 10, 2022

Conversation

srotondo
Copy link
Contributor

@srotondo srotondo commented Aug 8, 2022

@srotondo
Copy link
Contributor Author

srotondo commented Aug 8, 2022

While working on this, I also fixed a couple typos I noticed along the way.

# GH 47856
t = Timestamp("2022-01-01")
teststamp = t
s = Series([t])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Could you avoid one letter names?

@mroeschke mroeschke added Numeric Operations Arithmetic, Comparison, and Logical operations Timedelta Timedelta data type labels Aug 8, 2022
@@ -1157,7 +1157,9 @@ cdef class RelativeDeltaOffset(BaseOffset):
return dt64other
elif not self._use_relativedelta and hasattr(self, "_offset"):
# timedelta
delta = Timedelta(self._offset * self.n)
num_nano = getattr(self, "nanoseconds", 0)
rem_nano = Timedelta(nanoseconds=num_nano)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think for performance reasons, it would be good to only construct the Timedelta and add to the offset if nanoseconds != 0

@@ -1157,7 +1157,11 @@ cdef class RelativeDeltaOffset(BaseOffset):
return dt64other
elif not self._use_relativedelta and hasattr(self, "_offset"):
# timedelta
delta = Timedelta(self._offset * self.n)
if hasattr(self, "nanoseconds"):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Could you use the getattr and check for != 0 instead?

@mroeschke mroeschke added this to the 1.5 milestone Aug 10, 2022
@mroeschke mroeschke merged commit 9a88d42 into pandas-dev:main Aug 10, 2022
@mroeschke
Copy link
Member

Thanks @srotondo

noatamir pushed a commit to noatamir/pandas that referenced this pull request Nov 9, 2022
…7856 (pandas-dev#48008)

* BUG: Fixed ignoring of nanoseconds when adding to series pandas-dev#47856

* Update doc/source/whatsnew/v1.5.0.rst

Co-authored-by: Matthew Roeschke <[email protected]>

* BUG: Renamed test variables pandas-dev#47856

* BUG: Changed added if-else for performance pandas-dev#47856

* Update pandas/_libs/tslibs/offsets.pyx

Co-authored-by: Matthew Roeschke <[email protected]>

* BUG: Used getattr for nano check pandas-dev#47856

Co-authored-by: Steven Rotondo <[email protected]>
Co-authored-by: Matthew Roeschke <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Numeric Operations Arithmetic, Comparison, and Logical operations Timedelta Timedelta data type
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: The nanoseconds in pd.DateOffset was ignored when add to pd.Series
3 participants