arrow-py / arrow

🏹 Better dates & times for Python
https://arrow.readthedocs.io
Apache License 2.0
8.71k stars 673 forks source link

Implement remaining datetime methods #840

Closed systemcatch closed 3 years ago

systemcatch commented 4 years ago

We need to implement toordinal() and fromordinal() along with the new timestamp() method.

Also our isoformat() method should be updated to include the timespec arg.

pgogri commented 3 years ago

Hi, I would like to work on this request and contribute to Arrow. Is this request still up for grabs?

jadchaar commented 3 years ago

It should be. Just keep in mind though that these methods are not supported on python <3.6, and our code base has not fully dropped support tor py27 and py35 just yet.

pgogri commented 3 years ago

I see, so the manner in which these methods interact with the rest of Arrow's codebase could be tricky. Is it important for these new methods to also work well with py27 and py35?

I wish to get a deeper understanding of the Arrow's codebase and its structure. Are there any suggestions or resources to help me do so?

jadchaar commented 3 years ago

Currently, we are supporting Python 2.7+, but we are planning to drop 2.7, 3.5 very soon. You can submit your PR with support for 3.6+, but it may take some time to get merged once we are fully committed to the deprecation.

As for resources, I recommend reading through the arrow docs (https://arrow.readthedocs.io/en/stable/) and becoming familiar with the primary API file arrow.py, which is where these functions will most likely go.

systemcatch commented 3 years ago

@pgogri now that the big Python 2.7/3.5 PR has been merged I think you should be good to go here.

pgogri commented 3 years ago

Hi, I had a clarification, so I found that below is currently possible:

>>> import arrow
>>> import datetime
>>> a = arrow.utcnow()
>>> a
<Arrow [2020-11-12T06:09:02.869023+00:00]>
>>> a_ord = a.toordinal()
>>> a_ord
737741

I was just wondering whether this means that arrow can already support the toordinal() method of datetime?

For fromordinal() method, I think we need to build support so that it can return an arrow object with the command arrow.fromordinal()?

>>> import arrow
>>> import datetime
>>> a = arrow.utcnow()
>>> a
<Arrow [2020-11-12T06:09:02.869023+00:00]>
>>> a_ord = a.toordinal()
>>> a_ord
737741
>>> arrow.fromordinal(a_ord)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: module 'arrow' has no attribute 'fromordinal'
>>> datetime.datetime.fromordinal(a_ord)
datetime.datetime(2020, 11, 12, 0, 0)
>>> a.datetime.fromordinal(a_ord)
datetime.datetime(2020, 11, 12, 0, 0)
>>>

Just looking for some clarification what are goals for the toordinal() and fromordinal() methods. Thanks.

EDIT: I checked the codebase and it looks like we already have the toordinal() method for arrow, but no fromordinal()

Also can you clarify about the new timestamp() method you mentioned? This is what the datetime documentation has to say: "Changed in version 3.6: The timestamp() method uses the fold attribute to disambiguate the times during a repeated interval." But I think that this should work without any changes to arrow?

systemcatch commented 3 years ago

Hey @pgogri no need to bother about the timestamp() stuff, we've already done that.

Seems like toordinal() works fine. For fromordinal we would normally integrate that with arrow.get() however this would conflict with unix timestamps. Let's do what you suggested.

>>> arrow.fromordinal(737741)
arrow.Arrow(2020, 11, 12, 0, 0 , 0)

@jadchaar any thoughts here?

pgogri commented 3 years ago

Hi @systemcatch , thanks for your reply, Won't it be more like:

>>> arrow.fromordinal(737741)
<Arrow [2020-11-12T00:00:00+00:00]>

Or does that not work? Can you share a bit more on what you mean "when you say this (integrating with arrow.get()) would conflict with unix timestamps"?

Also,

>>> arrow.Arrow(2020, 11, 12, 0, 0, 0)
<Arrow [2020-11-12T00:00:00+00:00]>

EDIT: Yes, We could indeed also return an arrow object just as you described above. It could have more uses. But I am bit unclear on how to achieve this, because it feels like we can just return Arrow object. If I am not mistaken.

krisfremen commented 3 years ago

Or does that not work? Can you share a bit more on what you mean "when you say this (integrating with arrow.get()) would conflict with unix timestamps"?

Technically, 737741 is a valid unix timestamp, I believe @systemcatch was referring to the fact it would create ambiguity in parsing ordinals vs UNIX timestamps.

>>> datetime.fromtimestamp(737741)
datetime.datetime(1970, 1, 9, 7, 55, 41)
>>> datetime.fromordinal(737741)
datetime.datetime(2020, 11, 12, 0, 0)
systemcatch commented 3 years ago

@pgogri you are right I provided the new __repr__

pgogri commented 3 years ago

Thanks for all the clarifications! I have started to work on this, I will provide an update soon.

jadchaar commented 3 years ago

We should also make sure we support the timespec argument for isoformat(): https://docs.python.org/3/library/datetime.html#datetime.datetime.isoformat.

Edit: oops did not notice this was already mentioned in our original issue :).

pgogri commented 3 years ago

Yes, am on it :)