add: world_clock.py and test_world_clock.py#222
add: world_clock.py and test_world_clock.py#222nir9696 wants to merge 17 commits intoPythonFreeCourse:developfrom
Conversation
app/routers/world_clock.py
Outdated
There was a problem hiding this comment.
We shouldn't manage our own database of timezones. Please use pytz.all_timezones or something familiar :)
There was a problem hiding this comment.
I have looked into several modules in Python, including pytz, and I can not see how they help here. I have invested a lot of time and energy in writing a code that supports large amount(!) of cities and places and not just the "timezone places" themselves.
the "Mini DB" I've created is not just a list of timezones - it maps timezones to countries and subcountries and crucial for the core of my code - Ability to search the equivalent time of a custom city or place.
yammesicka
left a comment
yammesicka
left a comment
There was a problem hiding this comment.
Added few more comments :)
|
Hi :) |
Codecov Report
@@ Coverage Diff @@
## develop #222 +/- ##
===========================================
- Coverage 99.38% 95.54% -3.84%
===========================================
Files 37 80 +43
Lines 1456 3727 +2271
===========================================
+ Hits 1447 3561 +2114
- Misses 9 166 +157
Continue to review full report at Codecov.
|
yammesicka
left a comment
yammesicka
left a comment
There was a problem hiding this comment.
Hi, I've added further insights.
yammesicka
left a comment
yammesicka
left a comment
There was a problem hiding this comment.
Great start, just few things before we can merge it
yammesicka
left a comment
yammesicka
left a comment
There was a problem hiding this comment.
Great job, please take a look at the insights :)
| return city_element[SUBCOUNTRY_NAME_KEY_IN_CITIES] | ||
|
|
||
|
|
||
| async def get_api_data(url: str) -> Optional[List[Any]]: |
There was a problem hiding this comment.
Cache this function to prevent high loading times
There was a problem hiding this comment.
I did my best to do so by using the decorator @functools.lru_cache but I always get runtime errors with the message: "cannot reuse already awaited coroutine". The truth is I'm pretty desperate and do not know how to solve this. I probably won't have the time to work on the code again by the end of the submission deadline - Can you consider giving up this matter or help me here? I have worked so hard...
app/internal/world_clock.py
Outdated
There was a problem hiding this comment.
Use git precommit hooks to fix this formatting
There was a problem hiding this comment.
I don't see it. this note has already been resolved and I see the correct formmating without the \ char.
This note is also "Outdated". Can you check again? :)
app/resources/country-continent.json
Outdated
There was a problem hiding this comment.
Keys should be lowercase, reformat in a human readable way.
Also, is there really no python module that does this? I find this very very unlikely...
| return city_element[SUBCOUNTRY_NAME_KEY_IN_CITIES] | ||
|
|
||
|
|
||
| async def get_api_data(url: str) -> Optional[List[Any]]: |
| return continent_name | ||
|
|
||
|
|
||
| def load_country_continent_data_set() -> List[Dict[str, str]]: |
There was a problem hiding this comment.
Cache the return value, reading and parsing JSON every time is expensive
| ) | ||
|
|
||
|
|
||
| def load_city_country_data_set() -> List[Dict[str, str]]: |
There was a problem hiding this comment.
Cache the return value, reading and parsing JSON every time is expensive
| return f"{TIMEZONES_BASE_URL}/{timezone}" | ||
|
|
||
|
|
||
| def load_country_subcountry_data_set() -> List[Dict[str, str]]: |
|
|
There are few comments that aren't related to caching. Fix them and I'll consider this as a ticket :) |
4 participants
No description provided.