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

Change the IN_WE parser to consume data from a new API from the same data source #6115

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

Conversation

saurabhchatterjee23
Copy link
Contributor

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

  • I have tested my parser changes locally with poetry run test_parser "zone_key"
  • poetry run test_parser IN-WE consumption
  • poetry run test_parser "IN-NO->IN-WE" exchange
  • I have run pnpx prettier --write . and poetry run format to format my changes.

@saurabhchatterjee23
Copy link
Contributor Author

I ran the older and newer version of parser at 12pm IST today.
I compared the difference between the output of the older and the newer version
https://jsoncompare.com/#!/diff/id=a6fbddbdc41dc41ab35bee2e9548cb2b&fullscreen/

The data for the first 12 hours of the day matches precisely.
The new parser doesn't generate data beyond 12 pm because there is no data available from the source, the older version did generate data in the future because of a bug in the code

parsers/IN_WE.py Show resolved Hide resolved
parsers/IN_WE.py Show resolved Hide resolved
@ghost
Copy link

ghost commented Nov 9, 2023

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!

Copy link
Member

@VIKTORVAV99 VIKTORVAV99 left a 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.

parsers/IN_WE.py Show resolved Hide resolved
parsers/IN_WE.py Outdated Show resolved Hide resolved
parsers/IN_WE.py Show resolved Hide resolved
@saurabhchatterjee23
Copy link
Contributor Author

saurabhchatterjee23 commented Nov 10, 2023

I will wait for these changes to be pulled in before creating the next PR, does that sound reasonable?

Copy link
Member

@VIKTORVAV99 VIKTORVAV99 left a 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.

parsers/IN_WE.py Outdated Show resolved Hide resolved
parsers/IN_WE.py Outdated Show resolved Hide resolved
parsers/IN_WE.py Outdated Show resolved Hide resolved
parsers/IN_WE.py Outdated Show resolved Hide resolved
parsers/IN_WE.py Outdated Show resolved Hide resolved
parsers/IN_WE.py Outdated Show resolved Hide resolved
@saurabhchatterjee23
Copy link
Contributor Author

A few more comments that needs to be addressed before we can merge this.

Done

@saurabhchatterjee23
Copy link
Contributor Author

@VIKTORVAV99 Do you have any update on this ?

@VIKTORVAV99 VIKTORVAV99 self-requested a review November 21, 2023 09:40
@VIKTORVAV99
Copy link
Member

@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.

@saurabhchatterjee23 saurabhchatterjee23 requested a review from a user November 21, 2023 09:47
@saurabhchatterjee23
Copy link
Contributor Author

@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.

parsers/IN_WE.py Outdated Show resolved Hide resolved
Setting timezone for generating date for request param

Co-authored-by: Viktor Andersson <[email protected]>
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