#9 refactoring, CI #12

Merged
Idvon merged 10 commits from issues-9 into main 2023-03-19 14:50:12 +00:00
Member
No description provided.
d requested changes 2023-03-14 12:15:45 +00:00
@ -27,3 +14,1 @@
return printing(open_weather.weather_data(city, appid), args.output)
elif config_data['weather_provider']['name'] == "openmeteo":
return printing(open_meteo.weather_data(city), args.output)
parser = create_parser(args.config) # call parser
Owner

предлагаю назвать config_parser чтобы не путать потом с другими парсерами (погодными например)

предлагаю назвать `config_parser` чтобы не путать потом с другими парсерами (погодными например)
Idvon marked this conversation as resolved
@ -30,2 +18,3 @@
return city_data
else:
return "Please, check provider name"
if weather_config['provider'] == "openweather":
Owner

вынести эти ифы в логику провайдера

вынести эти ифы в логику провайдера
Idvon marked this conversation as resolved
@ -0,0 +7,4 @@
class ConfigFileParser:
config: dict
def get_geo_config(self) -> dict: # get geo_providers data
Owner

не очень информативные комменты

не очень информативные комменты
Idvon marked this conversation as resolved
@ -0,0 +29,4 @@
self.config = toml.load(f) # load toml from f(file)
extensions = {'.json': JSONParser,
Owner

Имена глобальных переменных обычно пишут БОЛЬШИМИ БУКВАМИ

Имена глобальных переменных обычно пишут БОЛЬШИМИ БУКВАМИ
Idvon marked this conversation as resolved
@ -0,0 +34,4 @@
def create_parser(file_name: str) -> ConfigFileParser:
extension = pathlib.Path(file_name).suffix
Owner

Предлагаю вынести создание Path в мейн и в create_parser передавать уже готовый file_path: Path. Потому что в теории create_parser должен заниматься только созданием парсера, а при создании объекта Path могут возникнуть ошибки (нет такого файла итд.), обрабатывать которые не его дело

Предлагаю вынести создание Path в мейн и в `create_parser` передавать уже готовый `file_path: Path`. Потому что в теории `create_parser` должен заниматься только созданием парсера, а при создании объекта `Path` могут возникнуть ошибки (нет такого файла итд.), обрабатывать которые не его дело
Idvon marked this conversation as resolved
@ -0,0 +26,4 @@
f"q={geo_config['city_name']}&"
f"appid={geo_config['api_key']}")
data = json.loads(call.text)
if type(data) is list:
Owner

Вроде как правильный способ проверки типа это
isinstance(data, list)

Вроде как правильный способ проверки типа это `isinstance(data, list)`
Idvon marked this conversation as resolved
@ -0,0 +35,4 @@
extensions = {'openweather': OpenWeatherGeoProvider}
def geo(geo_config: dict) -> GeoProvider:
Owner

Не понятно из названия, что за geo. Предлагаю назвать также как сделано в конфигпарсере. Хотя бы create_geo, а лучше create_geo_provider

Не понятно из названия, что за `geo`. Предлагаю назвать также как сделано в конфигпарсере. Хотя бы `create_geo`, а лучше `create_geo_provider`
Idvon marked this conversation as resolved
d changed title from issues-9 to WIP: issues-9 2023-03-14 17:01:40 +00:00
d added 1 commit 2023-03-18 13:52:35 +00:00
CI: add mypy, split to multiple steps
Some checks failed
ci/woodpecker/push/woodpecker Pipeline failed
26a9d6e8db
d force-pushed issues-9 from 26a9d6e8db
Some checks failed
ci/woodpecker/push/woodpecker Pipeline failed
to 014a30e96f
Some checks failed
ci/woodpecker/push/woodpecker Pipeline failed
2023-03-18 13:58:19 +00:00
Compare
ReFaCtOrInG
Some checks failed
ci/woodpecker/push/woodpecker Pipeline failed
23e432b11d
d added 1 commit 2023-03-19 05:43:17 +00:00
poetry install no root
Some checks failed
ci/woodpecker/push/woodpecker Pipeline failed
1e094e21c0
d added 1 commit 2023-03-19 05:59:16 +00:00
fix mypy
Some checks failed
ci/woodpecker/push/woodpecker Pipeline failed
63171a0933
d requested changes 2023-03-19 06:13:06 +00:00
@ -31,1 +15,3 @@
return "Please, check provider name"
if not Path(args.config).is_file():
return "Config file not found"
extension = Path(args.config).suffix
Owner

А зачем два раза создавать Path? Предлагаю просто один раз создать его и передать в функцию, а там уже вызывать у него .suffix

А зачем два раза создавать Path? Предлагаю просто один раз создать его и передать в функцию, а там уже вызывать у него .suffix
Idvon marked this conversation as resolved
@ -32,0 +29,4 @@
return weather_provider
weather_data = weather_provider.weather_data(weather_provider.request())
if isinstance(weather_data, str):
return weather_data
Owner

Интересный способ написания Exceptions ты изобрел))

Интересный способ написания Exceptions ты изобрел))
Author
Member

Ну так было быстрее чем Exceptions...

Ну так было быстрее чем Exceptions...
@ -0,0 +7,4 @@
def printing(geo_data, weather_data: Optional[dict], out_name: str) -> str:
date = {"datetime": datetime.datetime.now(datetime.timezone.utc)}
data = date | weather_data | geo_data
col = None if Path(out_name).is_file() else data.keys()
Owner

имя переменной не содержательное

имя переменной не содержательное
Author
Member

Какой из?)

Какой из?)
d marked this conversation as resolved
@ -0,0 +8,4 @@
date = {"datetime": datetime.datetime.now(datetime.timezone.utc)}
data = date | weather_data | geo_data
col = None if Path(out_name).is_file() else data.keys()
with open(out_name, "a", newline="") as f:
Owner

Ты можешь передавать в open() объект типа Path напрямую

Ты можешь передавать в open() объект типа Path напрямую
Author
Member

Не понял, а зачем?

Не понял, а зачем?
d marked this conversation as resolved
d changed title from WIP: issues-9 to WIP: #9 refactoring, CI 2023-03-19 07:58:15 +00:00
fix typing
Some checks failed
ci/woodpecker/push/woodpecker Pipeline failed
6321f6063d
fix typing
All checks were successful
ci/woodpecker/push/woodpecker Pipeline was successful
ba19af6c3c
Idvon changed title from WIP: #9 refactoring, CI to #9 refactoring, CI 2023-03-19 14:46:43 +00:00
d approved these changes 2023-03-19 14:48:55 +00:00
Idvon merged commit 8ca88b6cd9 into main 2023-03-19 14:50:11 +00:00
Idvon referenced this pull request from a commit 2023-03-19 14:50:12 +00:00
Idvon deleted branch issues-9 2023-03-19 14:50:12 +00:00
Sign in to join this conversation.
No description provided.