Skip to content

Conversation

@lithomas1
Copy link
Contributor

# self._value < other, we would get 0 as a result,
# which is not correct.
# We need to try going to a higher resolution to fix this
curr_value = self._value
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not the happiest with this code, since there's a possibility we might overflow (if other is very big), and there could be the possibility that a Tick is not representable in nanoseconds.

Is this a real possibility?
(not too familiar with tick code, so I can't tell mysel)

Copy link
Member

Choose a reason for hiding this comment

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

this introduces (untested) changes in Timedelta division in addition to Tick. I'm skeptical that this is the right place for a change, but if it is, it should be tested.

@lithomas1 lithomas1 added the Frequency DateOffsets label Feb 19, 2024
@lithomas1 lithomas1 added this to the 2.2.1 milestone Feb 20, 2024
@jbrockmendel
Copy link
Member

did you look at Tick._next_higher_resolution? it is what we use in Tick.__mul__ when multiplying by a float, and i suspect that logic can be adapted pretty cleanly

@lithomas1 lithomas1 modified the milestones: 2.2.1, 2.2.2 Feb 23, 2024
@github-actions
Copy link
Contributor

This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this.

@github-actions github-actions bot added the Stale label Mar 25, 2024
@lithomas1 lithomas1 removed this from the 2.2.2 milestone Apr 9, 2024
@mroeschke
Copy link
Member

Looks like this PR has gone stale and needs a rebase. Closing to clear the queue but feel free to reopen when you have time to circle back

@mroeschke mroeschke closed this Apr 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BUG: pandas.tseries.offsets.Second() division

3 participants