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

Add fetch_price method in GB parsers based on fetch_price FR parser #3709

Merged
merged 18 commits into from
Jan 27, 2022
Merged

Add fetch_price method in GB parsers based on fetch_price FR parser #3709

merged 18 commits into from
Jan 27, 2022

Conversation

AlexandreCouderc
Copy link
Contributor

GB prices are available in the data used by the FR parser to fetch prices for FR.

Copy link
Member

@madsnedergaard madsnedergaard left a comment

Choose a reason for hiding this comment

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

Haha, nice "hack" to use French data for getting GB prices 😄

madsnedergaard
madsnedergaard approved these changes Jan 27, 2022
Copy link
Member

@madsnedergaard madsnedergaard left a comment

Choose a reason for hiding this comment

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

Okay, I was a bit too fast here - there's a few chances required before we can merge this :)

  1. The parsers you have changed here are for Northern Ireland and the Orkney Islands. We should probably create a new file for GB only
  2. The zones.json file needs to be updated to use this new parser for price

Copy link
Member

@madsnedergaard madsnedergaard left a comment

Choose a reason for hiding this comment

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

Brilliant, thanks for the very quick iteration 🙌

@madsnedergaard
Copy link
Member

Closes #3706 and #2922 🥳

@madsnedergaard madsnedergaard merged commit 86ad87a into electricitymaps:master Jan 27, 2022
if donnesMarche.tag != 'donneesMarche':
continue

start_date = arrow.get(arrow.get(donnesMarche.attrib['date']).datetime, 'Europe/London')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have a doubt here if I should put 'Europe/London' or 'Europe/Paris' for the timezone.

Copy link
Member

Choose a reason for hiding this comment

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

That is a good question 😄
Let's see how the data looks now and then we can follow-up on it if it's clear that it's off :)

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.

2 participants