search

arrow-py / arrow

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

Use strptime tokens internally for parsing ISO 8601 week dates #857

Closed systemcatch closed 3 years ago

systemcatch commented 4 years ago

Available once we drop Python 2.7

https://github.com/arrow-py/arrow/blob/master/arrow/parser.py#L414

yiransii commented 3 years ago

Hello, I am interested in working on this as well.

systemcatch commented 3 years ago

Hi @yiransii now that 2.7/3.5 are dropped this is doable, shall I assign you?

yiransii commented 3 years ago

@systemcatch yea sure

yiransii commented 3 years ago

@systemcatch hello, I am a bit confused with exactly where to use datetime.strptime(). Could you please clarify?

Under the function _build_datetime delcaration, there is a comment on line 420 about using strptime(), which I think is for line 421 year, week = int(weekdate[0]), int(weekdate[1]) However, I did some testing and it seems that weekdate is already a tuple: 'weekdate': ('2011', '05', '4'). So line 421 makes sense w/o using datetime.strptime(). Am I misunderstanding something?

systemcatch commented 3 years ago

Hey @yiransii my idea is to do something similar to https://github.com/arrow-py/arrow/blob/master/arrow/parser.py#L464

That way there's no need for the iso_to_gregorian util function in the parser or https://github.com/arrow-py/arrow/blob/master/arrow/factory.py#L206

Does that make sense?

yiransii commented 3 years ago

@systemcatch

Thank you so much for the clarification.

I have updated parser.py and submitted a pull request. I have some dependency issues running tests locally but it has passed all tests as it shows in the pull request. https://github.com/arrow-py/arrow/pull/889

Please review my pull request and let me know if anything is off. I can work on https://github.com/arrow-py/arrow/blob/master/arrow/factory.py#L206 as well.