electricitymaps-contrib icon indicating copy to clipboard operation
electricitymaps-contrib copied to clipboard

Remove usage of arrow

Open VIKTORVAV99 opened this issue 1 year ago • 18 comments

Description

The built in datetime class is faster and less error prone than arrow so we should migrate to fully using datetime instead of arrow in our parsers.

This probably needs to be done in steps so it's easier to review as it can have a big impact on how parsers are structured.

Parsers missing:

Already done:

VIKTORVAV99 avatar Nov 12 '23 17:11 VIKTORVAV99

Hi @VIKTORVAV99 , I would like to work on this.

Barissa-Imran avatar Nov 23 '23 21:11 Barissa-Imran

Hi @VIKTORVAV99 , I would like to work on this.

Sure thing!

We use arrow in a lot of places so just pick a parser/file and remove it. It should be replaced by datetime objects and I would prefer one PR per parser as it will allow for much faster reviews.

VIKTORVAV99 avatar Nov 23 '23 21:11 VIKTORVAV99

I've compiled the following list to keep track of all parsers that have been refactored and one's the have not. The command I used is:

grep -El '(import arrow|arrow)' parsers/*.py

or the below to get files without arrow:

grep -L -E '(import arrow|arrow)' parsers/*.py

Edit by @VIKTORVAV99: The list has been moved to the top level issue comment so the tracking works correctly.

Barissa-Imran avatar Nov 29 '23 13:11 Barissa-Imran

Hello, would like to help too! This is my first time contributing, started with the parser for my home country: https://github.com/electricitymaps/electricitymaps-contrib/pull/6246

yujia21 avatar Dec 13 '23 13:12 yujia21

Hello, would like to help too! This is my first time contributing, started with the parser for my home country: #6246

Thats great, thanks!

VIKTORVAV99 avatar Dec 13 '23 15:12 VIKTORVAV99

Hi, I have just created PR #6481 to remove usage of the arrow module from the PE parser.

gianantoniopini avatar Feb 14 '24 11:02 gianantoniopini

Hallo! I was planning to start trying to remove usage of the arrow module from the CL parser, unless somebody else is already working on it.

gianantoniopini avatar Feb 26 '24 12:02 gianantoniopini

Hallo! I was planning to start trying to remove usage of the arrow module from the CL parser, unless somebody else is already working on it.

I don't think anyone is working on that particular parser so go right ahead! And thanks!

VIKTORVAV99 avatar Feb 26 '24 13:02 VIKTORVAV99

Thanks!

gianantoniopini avatar Feb 26 '24 13:02 gianantoniopini

I tried to start on CA_BC this evening, but ran into an issue in attempting an initial run test_parser. Here is the full output: poetry run test_parser CA-BC Traceback (most recent call last): File "<string>", line 1, in <module> File "/workspaces/electricitymaps-contrib/.venv/lib/python3.10/site-packages/click/core.py", line 1134, in __call__ return self.main(*args, **kwargs) File "/workspaces/electricitymaps-contrib/.venv/lib/python3.10/site-packages/click/core.py", line 1059, in main rv = self.invoke(ctx) File "/workspaces/electricitymaps-contrib/.venv/lib/python3.10/site-packages/click/core.py", line 1401, in invoke return ctx.invoke(self.callback, **ctx.params) File "/workspaces/electricitymaps-contrib/.venv/lib/python3.10/site-packages/click/core.py", line 767, in invoke return __callback(*args, **kwargs) File "/workspaces/electricitymaps-contrib/test_parser.py", line 61, in test_parser parser: Callable[..., list[dict[str, Any]] | dict[str, Any]] = PARSER_KEY_TO_DICT[ KeyError: 'CA-BC'

I've tried all permutations of CA-BC with underscores and spaces, and still can't seem to find the key. The web app shows 'CA-BC', and the same zone key is listed multiple times in the parser code. Any idea on what might be causing this?

nboswell216 avatar Mar 22 '24 06:03 nboswell216

@nboswell216 When running poetry run test_parser without the second DATA_TYPE option, it looks like it will be the default production. But CA_BC.py doesn't have a fetch_production() function so it failed to run. CA_BC.py instead defines fetch_consumption() so consumption option can be run:

> poetry run test_parser CA-BC consumption
Parser result:
[{'consumption': 7194.0,
  'datetime': datetime.datetime(2024, 2, 20, 5, 0, tzinfo=zoneinfo.ZoneInfo(key='America/Vancouver')),
  'source': 'bchydro.com',
  'sourceType': <EventSourceType.measured: 'measured'>,
  'zoneKey': 'CA-BC'},

shuuji3 avatar Mar 23 '24 11:03 shuuji3

Hi, just wondering if I can still contribute to this issue. I'm first time trying to contribute, very beginner level. Do I just need to change parser to date time object? Would it be possible to see any before fix version and after fix version?

lin0110 avatar Apr 09 '24 17:04 lin0110

Hi, just wondering if I can still contribute to this issue. I'm first time trying to contribute, very beginner level. Do I just need to change parser to date time object? Would it be possible to see any before fix version and after fix version?

There are still a few parsers that need updating I think but if you want a before and after fix I would look at some of the merged PRs. There you should be able to see both the code before and after as well as the diff between the two.

VIKTORVAV99 avatar Apr 09 '24 17:04 VIKTORVAV99

Hi, just wondering if I can still contribute to this issue. I'm first time trying to contribute, very beginner level. Do I just need to change parser to date time object? Would it be possible to see any before fix version and after fix version?

There are still a few parsers that need updating I think but if you want a before and after fix I would look at some of the merged PRs. There you should be able to see both the code before and after as well as the diff between the two.

Thank you for clarifying! If no one is working on [parsers/IN.py], can I work on [parsers/IN.py]? Or is there any other suggested parser for beginner? :)

lin0110 avatar Apr 12 '24 20:04 lin0110

Hi, just wondering if I can still contribute to this issue. I'm first time trying to contribute, very beginner level. Do I just need to change parser to date time object? Would it be possible to see any before fix version and after fix version?

There are still a few parsers that need updating I think but if you want a before and after fix I would look at some of the merged PRs. There you should be able to see both the code before and after as well as the diff between the two.

Thank you for clarifying! If no one is working on [parsers/IN.py], can I work on [parsers/IN.py]? Or is there any other suggested parser for beginner? :)

@lin0110 Any parser will be good for a beginner. If you get stuck, check out how the conversion has been handled in other parsers, especially in the same region. No need to ask to work on any of the parsers for this issue - if you don't see any open PR for a parser, just go for it!

nboswell216 avatar Apr 13 '24 02:04 nboswell216

Hi, just wondering if I can still contribute to this issue. I'm first time trying to contribute, very beginner level. Do I just need to change parser to date time object? Would it be possible to see any before fix version and after fix version?

There are still a few parsers that need updating I think but if you want a before and after fix I would look at some of the merged PRs. There you should be able to see both the code before and after as well as the diff between the two.

Thank you for clarifying! If no one is working on [parsers/IN.py], can I work on [parsers/IN.py]? Or is there any other suggested parser for beginner? :)

@lin0110 Any parser will be good for a beginner. If you get stuck, check out how the conversion has been handled in other parsers, especially in the same region. No need to ask to work on any of the parsers for this issue - if you don't see any open PR for a parser, just go for it!

Thank you guys for responding and encouraging me, I really appreciate it! I'm currently working on IN.py, but I'm encountering a ConnectTimeoutError while running 'poetry run test_parser IN'. How can I fix this so I can continue testing my code?

lin0110 avatar Apr 19 '24 14:04 lin0110