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

refactor: removed arrow from CA-AB parser #6179

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Barissa-Imran
Copy link
Contributor

Issue

Ref: #6135

Description

Upgrade the parser to use datetime. Parser functionality has not changed.

  • add 24 hour handling (midnight next day)

Preview

Double check

  • I have tested my parser changes locally with poetry run test_parser "zone_key"
  • I have run pnpx prettier --write . and poetry run format to format my changes.

Upgrade the parser to use `datetime`. Parser functionality has not changed.

- add `24` hour handling (midnight next day)
@VIKTORVAV99 VIKTORVAV99 self-requested a review November 28, 2023 15:07
@Barissa-Imran Barissa-Imran mentioned this pull request Nov 29, 2023
95 tasks
parsers/CA_AB.py Outdated Show resolved Hide resolved
@VIKTORVAV99 VIKTORVAV99 self-requested a review December 11, 2023 17:40
Comment on lines +85 to +86
datetime=datetime.strptime(f"{date} {int(hour) - 1}", "%m/%d/%Y %H")
+ timedelta(hours=1),
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
datetime=datetime.strptime(f"{date} {int(hour) - 1}", "%m/%d/%Y %H")
+ timedelta(hours=1),
datetime=datetime.strptime(f"{date} {int(hour) - 1}", "%m/%d/%Y %H").replace(tzinfo=TIMEZONE),
+ timedelta(hours=1),

We need to attach a the timezone as well.

Also are you sure that the value of hour 24 are for 00 the next day and not simply hour 23 but 1 indexed? Otherwise we don't need the timedelta.

@VIKTORVAV99 VIKTORVAV99 self-requested a review December 22, 2023 16:24
@VIKTORVAV99 VIKTORVAV99 self-assigned this Jan 19, 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.

None yet

2 participants