-
Notifications
You must be signed in to change notification settings - Fork 45
Codecleanup #3 #54
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
base: main
Are you sure you want to change the base?
Codecleanup #3 #54
Changes from 10 commits
3d2ac32
8c1851b
b38375a
2ce2e6b
5105fd6
2f8ecc7
0066ff3
111aab0
bf310f6
acec795
02c421a
e3f6946
8fbb0c2
b6bcce9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -42,48 +42,55 @@ def arguments_dictionary(lat, long, city, args): | |||||||||||||||
| return arguments | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| def set_output_values(args, arguments): # noqa | ||||||||||||||||
| """ | ||||||||||||||||
| Takes a list of command line arguments(args) | ||||||||||||||||
| and sets the appropritate values | ||||||||||||||||
| in the arguments dictionary(show_wave = 1, etc). | ||||||||||||||||
| Returns the arguments dict with the updated CLI args | ||||||||||||||||
| """ | ||||||||||||||||
| if "hide_wave" in args or "hw" in args: | ||||||||||||||||
| arguments["show_wave"] = 0 | ||||||||||||||||
| if "show_large_wave" in args or "slw" in args: | ||||||||||||||||
| arguments["show_large_wave"] = 1 | ||||||||||||||||
| if "hide_uv" in args or "huv" in args: | ||||||||||||||||
| arguments["show_uv"] = 0 | ||||||||||||||||
| if "hide_height" in args or "hh" in args: | ||||||||||||||||
| arguments["show_height"] = 0 | ||||||||||||||||
| if "hide_direction" in args or "hdir" in args: | ||||||||||||||||
| arguments["show_direction"] = 0 | ||||||||||||||||
| if "hide_period" in args or "hp" in args: | ||||||||||||||||
| arguments["show_period"] = 0 | ||||||||||||||||
| if "hide_location" in args or "hl" in args: | ||||||||||||||||
| arguments["show_city"] = 0 | ||||||||||||||||
| if "hide_date" in args or "hdate" in args: | ||||||||||||||||
| arguments["show_date"] = 0 | ||||||||||||||||
| if "metric" in args or "m" in args: | ||||||||||||||||
| arguments["unit"] = "metric" | ||||||||||||||||
| if "json" in args or "j" in args: | ||||||||||||||||
| arguments["json_output"] = 1 | ||||||||||||||||
| if "gpt" in args or "g" in args: | ||||||||||||||||
| arguments["gpt"] = 1 | ||||||||||||||||
| if "show_air_temp" in args or "sat" in args: | ||||||||||||||||
| arguments["show_air_temp"] = 1 | ||||||||||||||||
| if "show_wind_speed" in args or "sws" in args: | ||||||||||||||||
| arguments["show_wind_speed"] = 1 | ||||||||||||||||
| if "show_wind_direction" in args or "swd" in args: | ||||||||||||||||
| arguments["show_wind_direction"] = 1 | ||||||||||||||||
| def set_output_values(args, arguments): | ||||||||||||||||
| """ | ||||||||||||||||
| Takes a list of command line arguments (args) | ||||||||||||||||
| and sets the appropriate values | ||||||||||||||||
| in the arguments dictionary (show_wave = 1, etc). | ||||||||||||||||
| Returns the arguments dict with the updated CLI args. | ||||||||||||||||
| """ | ||||||||||||||||
| actions = { | ||||||||||||||||
| "hide_wave": ("show_wave", 0), | ||||||||||||||||
| "hw": ("show_wave", 0), | ||||||||||||||||
| "show_large_wave": ("show_large_wave", 1), | ||||||||||||||||
| "slw": ("show_large_wave", 1), | ||||||||||||||||
| "hide_uv": ("show_uv", 0), | ||||||||||||||||
| "huv": ("show_uv", 0), | ||||||||||||||||
| "hide_height": ("show_height", 0), | ||||||||||||||||
| "hh": ("show_height", 0), | ||||||||||||||||
| "hide_direction": ("show_direction", 0), | ||||||||||||||||
| "hdir": ("show_direction", 0), | ||||||||||||||||
| "hide_period": ("show_period", 0), | ||||||||||||||||
| "hp": ("show_period", 0), | ||||||||||||||||
| "hide_location": ("show_city", 0), | ||||||||||||||||
| "hl": ("show_city", 0), | ||||||||||||||||
| "hide_date": ("show_date", 0), | ||||||||||||||||
| "hdate": ("show_date", 0), | ||||||||||||||||
| "metric": ("unit", "metric"), | ||||||||||||||||
| "m": ("unit", "metric"), | ||||||||||||||||
| "json": ("json_output", 1), | ||||||||||||||||
| "j": ("json_output", 1), | ||||||||||||||||
| "gpt": ("gpt", 1), | ||||||||||||||||
| "g": ("gpt", 1), | ||||||||||||||||
| "show_air_temp": ("show_air_temp", 1), | ||||||||||||||||
| "sat": ("show_air_temp", 1), | ||||||||||||||||
| "show_wind_speed": ("show_wind_speed", 1), | ||||||||||||||||
| "sws": ("show_wind_speed", 1), | ||||||||||||||||
| "show_wind_direction": ("show_wind_direction", 1), | ||||||||||||||||
| "swd": ("show_wind_direction", 1), | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| for arg in args: | ||||||||||||||||
| if arg in actions: | ||||||||||||||||
| key, value = actions[arg] | ||||||||||||||||
| arguments[key] = value | ||||||||||||||||
|
|
||||||||||||||||
| return arguments | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| def seperate_args(args): | ||||||||||||||||
| def separate_args(args): | ||||||||||||||||
| """ | ||||||||||||||||
| Args are seperated by commas in input. Sereperat them and return list | ||||||||||||||||
| Args are separated by commas in input. Separate them and return list | ||||||||||||||||
| """ | ||||||||||||||||
| if len(args) > 1: | ||||||||||||||||
| new_args = args[1].split(",") | ||||||||||||||||
|
|
@@ -112,44 +119,51 @@ def print_location(city, show_city): | |||||||||||||||
| Prints location | ||||||||||||||||
| """ | ||||||||||||||||
| if int(show_city) == 1: | ||||||||||||||||
| print("Location: ", city) | ||||||||||||||||
| print("\n") | ||||||||||||||||
| print("Location:", city) | ||||||||||||||||
| else: | ||||||||||||||||
| print("Location unknown") | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| def print_ocean_data(arguments_dict, ocean_data_dict): | ||||||||||||||||
| def print_ocean_data(arguments_dict, ocean_data): | ||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: Improve handling of None values in Instead of printing 'None' for missing values, consider using a more user-friendly message like 'Not available' or 'N/A'.
Suggested change
|
||||||||||||||||
| """ | ||||||||||||||||
| Prints ocean data(height, wave direction, period, etc) | ||||||||||||||||
| Prints ocean data (height, wave direction, period, etc.) | ||||||||||||||||
| """ | ||||||||||||||||
| if int(arguments_dict["show_uv"]) == 1: | ||||||||||||||||
| print("UV index: ", ocean_data_dict["UV Index"]) | ||||||||||||||||
| if int(arguments_dict["show_height"]) == 1: | ||||||||||||||||
| print("Wave Height: ", ocean_data_dict["Height"]) | ||||||||||||||||
| if int(arguments_dict["show_direction"]) == 1: | ||||||||||||||||
| print("Wave Direction: ", ocean_data_dict["Swell Direction"]) | ||||||||||||||||
| if int(arguments_dict["show_period"]) == 1: | ||||||||||||||||
| print("Wave Period: ", ocean_data_dict["Period"]) | ||||||||||||||||
| if int(arguments_dict["show_air_temp"]) == 1: | ||||||||||||||||
| print("Air Temp: ", ocean_data_dict["Air Temperature"]) | ||||||||||||||||
| if int(arguments_dict["show_wind_speed"]) == 1: | ||||||||||||||||
| print("Wind Speed: ", ocean_data_dict["Wind Speed"]) | ||||||||||||||||
| if int(arguments_dict["show_wind_direction"]) == 1: | ||||||||||||||||
| print("Wind Direction: ", ocean_data_dict["Wind Direction"]) | ||||||||||||||||
| display_mapping = { | ||||||||||||||||
| "show_uv": ("UV Index", "UV index: "), | ||||||||||||||||
| "show_height": ("Height", "Wave Height: "), | ||||||||||||||||
| "show_direction": ("Swell Direction", "Wave Direction: "), | ||||||||||||||||
| "show_period": ("Period", "Wave Period: "), | ||||||||||||||||
| "show_air_temp": ("Air Temperature", "Air Temp: "), | ||||||||||||||||
| "show_wind_speed": ("Wind Speed", "Wind Speed: "), | ||||||||||||||||
| "show_wind_direction": ("Wind Direction", "Wind Direction: "), | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| for key, (data_key, label) in display_mapping.items(): | ||||||||||||||||
| if int(arguments_dict.get(key, 0)) == 1: | ||||||||||||||||
| value = ocean_data.get(data_key) | ||||||||||||||||
| if value is not None: | ||||||||||||||||
| print(f"{label}{value}") | ||||||||||||||||
| else: | ||||||||||||||||
| print(f"{label} None") | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| def print_forecast(ocean, forecast): | ||||||||||||||||
| """ | ||||||||||||||||
| Takes in list of forecast data and prints | ||||||||||||||||
| """ | ||||||||||||||||
| transposed = list(zip(*forecast)) | ||||||||||||||||
|
|
||||||||||||||||
| for day in transposed: | ||||||||||||||||
| if ocean["show_date"] == 1: | ||||||||||||||||
| print("Date: ", day[3]) | ||||||||||||||||
| if int(ocean["show_height"]) == 1: | ||||||||||||||||
| print("Wave Height: ", day[0]) | ||||||||||||||||
| if int(ocean["show_direction"]) == 1: | ||||||||||||||||
| print("Wave Direction: ", day[1]) | ||||||||||||||||
| if int(ocean["show_period"]) == 1: | ||||||||||||||||
| print("Wave Period: ", day[2]) | ||||||||||||||||
| actions = { | ||||||||||||||||
| "show_date": (3, "Date: "), | ||||||||||||||||
| "show_height": (0, "Wave Height: "), | ||||||||||||||||
| "show_direction": (1, "Wave Direction: "), | ||||||||||||||||
| "show_period": (2, "Wave Period: "), | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| for key, (index, forecast_data) in actions.items(): | ||||||||||||||||
| if int(ocean.get(key, 0)) == 1: | ||||||||||||||||
| print(forecast_data, day[index]) | ||||||||||||||||
| print("\n") | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -5,9 +5,11 @@ | |||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| import io | ||||||||||||||||||||||||||||||
| import sys | ||||||||||||||||||||||||||||||
| from io import StringIO | ||||||||||||||||||||||||||||||
| from unittest.mock import patch | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| from src.helper import extract_decimal | ||||||||||||||||||||||||||||||
| from src.helper import extract_decimal, print_location, set_output_values, print_ocean_data | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| def test_invalid_input(): | ||||||||||||||||||||||||||||||
|
|
@@ -27,3 +29,55 @@ def test_default_input(): | |||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||
| decimal = extract_decimal([]) | ||||||||||||||||||||||||||||||
| assert 1 == decimal | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| def test_print_location(): | ||||||||||||||||||||||||||||||
| city = "Perth" | ||||||||||||||||||||||||||||||
| show_city = 1 | ||||||||||||||||||||||||||||||
| captured_output = io.StringIO() | ||||||||||||||||||||||||||||||
| sys.stdout = captured_output | ||||||||||||||||||||||||||||||
| print_location(city, show_city) | ||||||||||||||||||||||||||||||
| sys.stdout = sys.__stdout__ | ||||||||||||||||||||||||||||||
| expected_output = "Location: Perth" | ||||||||||||||||||||||||||||||
| assert captured_output.getvalue().strip() == expected_output.strip() | ||||||||||||||||||||||||||||||
|
Comment on lines
+34
to
+52
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion (testing): Test for print_location does not cover the case when show_city is 0 Please add a test case for when |
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| def test_set_output_values(): | ||||||||||||||||||||||||||||||
| args = ['hw', 'show_large_wave', 'huv'] | ||||||||||||||||||||||||||||||
| arguments = {} | ||||||||||||||||||||||||||||||
| expected = {"show_wave": 0, "show_large_wave": 1, "show_uv": 0} | ||||||||||||||||||||||||||||||
| assert set_output_values(args, arguments) == expected | ||||||||||||||||||||||||||||||
|
Comment on lines
+45
to
+78
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion (testing): Test for set_output_values does not cover all possible arguments Consider adding more test cases to cover all possible arguments in the
Suggested change
|
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| def test_print_ocean_data(): | ||||||||||||||||||||||||||||||
| arguments_dict = { | ||||||||||||||||||||||||||||||
| "show_uv": "1", | ||||||||||||||||||||||||||||||
| "show_height": "1", | ||||||||||||||||||||||||||||||
| "show_direction": "1", | ||||||||||||||||||||||||||||||
| "show_period": "0", | ||||||||||||||||||||||||||||||
| "show_air_temp": "1", | ||||||||||||||||||||||||||||||
| "show_wind_speed": "1", | ||||||||||||||||||||||||||||||
| "show_wind_direction": "0" | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
Comment on lines
+52
to
+94
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion (testing): Test for print_ocean_data does not cover all display_mapping keys Please add test cases to cover all keys in the |
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| ocean_data = { | ||||||||||||||||||||||||||||||
| "UV Index": 5, | ||||||||||||||||||||||||||||||
| "Height": 2.5, | ||||||||||||||||||||||||||||||
| "Swell Direction": "NE", | ||||||||||||||||||||||||||||||
| "Air Temperature": 25, | ||||||||||||||||||||||||||||||
| "Wind Speed": 15 | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| expected_output = ( | ||||||||||||||||||||||||||||||
| "UV index: 5\n" | ||||||||||||||||||||||||||||||
| "Wave Height: 2.5\n" | ||||||||||||||||||||||||||||||
| "Wave Direction: NE\n" | ||||||||||||||||||||||||||||||
| "Air Temp: 25\n" | ||||||||||||||||||||||||||||||
| "Wind Speed: 15\n" | ||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| captured_output = StringIO() | ||||||||||||||||||||||||||||||
| sys.stdout = captured_output | ||||||||||||||||||||||||||||||
| print_ocean_data(arguments_dict, ocean_data) | ||||||||||||||||||||||||||||||
| sys.stdout = sys.__stdout__ | ||||||||||||||||||||||||||||||
| assert captured_output.getvalue() == expected_output | ||||||||||||||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work!
IMO, there's one point I'd like to highlight. To ensure that the modifications you've made haven't caused any regressions (and that everything is functioning correctly), I recommend adding test cases to
test_helper.py. This will help guarantee the integrity of the changes.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @ryansurf Thank you!
I only made changes to the helper.py function and cli.py function where I fixed a typo. I am not sure how files like idea/misc.xml were altered.
I will also try to figure out why the formatter is failing but if not, I will let you know
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! @K-dash
Yes I agree with you on that. I'm adding some tests to the test_helper.py file. Will push the changes for your review when I am done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When you do
git add <file>, make sure you're only addinghelper.pyandcli.py!There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @GUKWAT, hows it going? Feel free to ping me if you need any help!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @ryansurf I am just working on the unit tests for helper.py. Still trying to figure out how to format the code using ruff having issues when it comes to the file path
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@GUKWAT great! What are the file path errors you are dealing with?