-
Notifications
You must be signed in to change notification settings - Fork 910
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
Change the IN_WE parser to consume data from a new API from the same data source #6115
base: master
Are you sure you want to change the base?
Change the IN_WE parser to consume data from a new API from the same data source #6115
Conversation
I ran the older and newer version of parser at 12pm IST today. The data for the first 12 hours of the day matches precisely. |
Hi @saurabhchatterjee23, thank you for breaking up your PR in smaller chunks. It is much easier to review and to see how the changes impact the parser. I have made a comment about not return 0 when data is missing but other than that, this looks great! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with @mathilde-daugy comments but I have some additional notes as well.
I will wait for these changes to be pulled in before creating the next PR, does that sound reasonable? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few more comments that needs to be addressed before we can merge this.
Done |
@VIKTORVAV99 Do you have any update on this ? |
I did not see that you had made changes until you pinged me. A good practice would be to re-request a review after changes have been made (request for reviews have different notification settings and workflow integrations). But I'll take a look at this later today if I have the time. |
Thanks for letting me know, I am a bit new to GitHub reviews. |
Setting timezone for generating date for request param Co-authored-by: Viktor Andersson <[email protected]>
Issue
This PR breaks down the changes in #6095 into smaller incremental PRs
Description
This PR is to change the IN_WE parser to consume data from a new API from the same data source which provides data in 24-hour format using the same algorithm used in the older version
Preview
Double check
poetry run test_parser "zone_key"
pnpx prettier --write .
andpoetry run format
to format my changes.