Fix docstring timestamps (Issue #59458)
- [x] Addresses some tasks of #59458
- [ ] Tests added and passed if fixing a bug or adding a new feature
- [x] All code checks passed.
- [x] Added type annotations to new arguments/methods/functions.
- [ ] Added an entry in the latest
doc/source/whatsnew/vX.X.X.rstfile if fixing a bug or adding a new feature.
thanks @mimistiv ! 🙏
i'll take a closer look later, but I think that year might need defining in some other places - i'd suggest checking how/where month is defined and taking inspiration from there
or just splitting year off from this PR and doing it separately, so we can merge month here quickly
thanks @mimistiv ! 🙏
i'll take a closer look later, but I think that
yearmight need defining in some other places - i'd suggest checking how/where month is defined and taking inspiration from thereor just splitting
yearoff from this PR and doing it separately, so we can mergemonthhere quickly
thanks @MarcoGorelli ! I think the issue is related to https://github.com/pandas-dev/pandas/blob/main/pandas/_libs/tslibs/timestamps.pxd#L24, where nanosecond and year are already declared
hey @MarcoGorelli! Do you have any advice on how I could best handle this issue? I was thinking I could change the declarations in the pxd file to use _year and _nanosecond similarly to _value but that will mean I need to change it everywhere and I need to be sure what all the uses are. In the meantime I might create a separate PR to handle the value changes and the month small fix.
hey - sure, let's start with a separate pr which we can merge quickly, and then we'll iterate over the trickier one?
hey - sure, let's start with a separate pr which we can merge quickly, and then we'll iterate over the trickier one?
Hey @MarcoGorelli , I split up the PR and the value/month part of it has now been merged! :) Could you please take a look at the suggested solution I have for year and nanosecond here?
Hey @MarcoGorelli @mroeschke, could I ask one of you to please take a look at the proposed solution in this PR when you have the time? Thanks in advance!
hey - sure, sorry for the delay, i still need to undertsand why year needed to be marked as cdef readonly in the first place, it's already a property of the python standard library's datetime.datetime...will get back to you on this
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.
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.
my bad unfortunately, just had a work trip + holiday, and very little / no funded time at the moment - i'll get back to this, i'll remove the 'stale' label
Thanks @MarcoGorelli ! I can see that merging with main resulted in a failed unit test which seems unrelated to my PR. Is this a known issue?
agree that it's unrelated, but recent commits look green 🥦 so it may have just fixed itself - let's ship this then, thanks for your contribution!