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

fix + upgrade LK parser and rename it #5934

Merged
merged 15 commits into from
Oct 19, 2023
Merged

Conversation

VIKTORVAV99
Copy link
Member

@VIKTORVAV99 VIKTORVAV99 commented Sep 22, 2023

Issue

The parser didn't use the new parser classes and where missing tests.

Description

  • Upgraded the parser to use parser classes
  • Removed arrow
  • Renamed parser
  • Added tests
  • Add historical parsing (data source only have about of month of historical data though), this also fixes the current LK issue.

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.

@github-actions github-actions bot added parser tests zone config Pull request or issue for zone configurations labels Sep 22, 2023
@VIKTORVAV99
Copy link
Member Author

not really sure why the test is failing here, it's passing for me locally. I would appreciate if you could take a look.

@VIKTORVAV99 VIKTORVAV99 marked this pull request as draft September 25, 2023 15:43
@VIKTORVAV99 VIKTORVAV99 changed the title upgrade LK parser and rename it fix + upgrade LK parser and rename it Sep 25, 2023
@VIKTORVAV99 VIKTORVAV99 marked this pull request as ready for review September 25, 2023 16:12
parsers/CEB.py Outdated Show resolved Hide resolved
Copy link
Contributor

@unitrium unitrium left a comment

Choose a reason for hiding this comment

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

Looks good thanks for handling this!

parsers/CEB.py Outdated Show resolved Hide resolved
@q--
Copy link
Contributor

q-- commented Oct 17, 2023

Could the test error have anything to do with time zone settings? A difference of two hours is exactly the difference between Central European Summer Time, which I presume your PC is set to, and UTC.

@VIKTORVAV99
Copy link
Member Author

Could the test error have anything to do with time zone settings? A difference of two hours is exactly the difference between Central European Summer Time, which I presume your PC is set to, and UTC.

Could be it but I thought I made sure all date times where time zone aware and only used UTC. I'll have to double check though.

@VIKTORVAV99 VIKTORVAV99 merged commit 9e7ec47 into master Oct 19, 2023
17 checks passed
@VIKTORVAV99 VIKTORVAV99 deleted the vik/upgrade_LK_parser branch October 19, 2023 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
parser tests zone config Pull request or issue for zone configurations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants