Codecleanup #3 #54

GUKWAT · 2024-07-23T07:26:57Z

For better code readability and maintenance, I changed the if statements into dictionary mapping conditions

Summary by Sourcery

This pull request refactors several functions to use dictionary mappings for improved readability and maintainability. It also fixes a typo in a function name and adds unit tests for key functions.

Bug Fixes:
- Fixed typo in function name seperate_args to separate_args in helper.py and cli.py.
Enhancements:
- Refactored set_output_values function to use dictionary mapping for better readability and maintainability.
- Refactored print_ocean_data function to use dictionary mapping for displaying ocean data.
- Refactored print_forecast function to use dictionary mapping for displaying forecast data.
Tests:
- Added unit tests for print_location, set_output_values, and print_ocean_data functions in test_helper.py.

ryansurf · 2024-07-23T16:57:51Z

Hey @GUKWAT, nice job! I'll review your PR this afternoon.

In the meantime, the formatter failed... could you try to resolve those errors? You should be able to run poetry run pre-commit run --all-files in your local environment to see whats going on.

If not, I can give you a hand later. Regardless, thank you for your contribution

Also, I think you may have committed some files that shouldn't have been! I see 10 files changed in the PR, like .idea/misc.xml. Shouldn't only helper.py be altered? I can help on this too, unless I'm missing something

K-dash · 2024-07-24T00:21:15Z

src/helper.py

+    for arg in args:
+        if arg in actions:
+            key, value = actions[arg]
+            arguments[key] = value


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.

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

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.

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

When you do git add <file>, make sure you're only adding helper.py and cli.py!

Hey @GUKWAT, hows it going? Feel free to ping me if you need any help!

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

@GUKWAT great! What are the file path errors you are dealing with?

ryansurf · 2024-07-29T18:24:07Z

@sourcery-ai review

sourcery-ai · 2024-07-29T18:24:14Z

Reviewer's Guide by Sourcery

This pull request focuses on improving code readability and maintainability by refactoring conditional logic in the src/helper.py file to use dictionary mappings. Additionally, it corrects spelling mistakes in function names and comments, and adds new test cases in tests/test_helper.py to ensure the correctness of the refactored functions.

File-Level Changes

Files	Changes
`src/helper.py` `tests/test_helper.py`	Refactored functions to use dictionary mappings for conditions and added corresponding test cases.
`src/helper.py` `src/cli.py`	Corrected spelling mistakes in function names and comments.

Tips

Trigger a new Sourcery review by commenting @sourcery-ai review on the pull request.
Continue your discussion with Sourcery by replying directly to review comments.
You can change your review settings at any time by accessing your dashboard:
- Enable or disable the Sourcery-generated pull request summary or reviewer's guide;
- Change the review language;
You can always contact us if you have any questions or feedback.

sourcery-ai

Hey @GUKWAT - I've reviewed your changes and they look great!

Here's what I looked at during the review

🟡 General issues: 1 issue found
🟢 Security: all looks good
🟡 Testing: 3 issues found
🟢 Complexity: all looks good
🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

_{Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.}

sourcery-ai · 2024-07-29T18:25:41Z

src/helper.py



-def print_ocean_data(arguments_dict, ocean_data_dict):
+def print_ocean_data(arguments_dict, ocean_data):


suggestion: Improve handling of None values in print_ocean_data

Instead of printing 'None' for missing values, consider using a more user-friendly message like 'Not available' or 'N/A'.

Suggested change

def print_ocean_data(arguments_dict, ocean_data):

def print_ocean_data(arguments_dict, ocean_data):

for key, value in ocean_data.items():

if value is None:

print(f"{key}: N/A")

else:

print(f"{key}: {value}")

sourcery-ai · 2024-07-29T18:25:42Z

tests/test_helper.py

+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()


suggestion (testing): Test for print_location does not cover the case when show_city is 0

Please add a test case for when show_city is 0 to ensure that the function handles this scenario correctly.

sourcery-ai · 2024-07-29T18:25:42Z

tests/test_helper.py

+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


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 actions dictionary to ensure comprehensive testing.

Suggested change

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

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

args = ['show_wave', 'show_uv']

expected = {"show_wave": 1, "show_large_wave": 0, "show_uv": 1}

assert set_output_values(args, arguments) == expected

sourcery-ai · 2024-07-29T18:25:42Z

tests/test_helper.py

+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"
+    }


suggestion (testing): Test for print_ocean_data does not cover all display_mapping keys

Please add test cases to cover all keys in the display_mapping dictionary to ensure that all possible outputs are tested.

GUKWAT added 4 commits July 21, 2024 13:37

set_outout_values function refactored

3d2ac32

print_forecast function refactored

8c1851b

fixed typo function seperate_args to separate_args

b38375a

print_ocean_data method refactored

2ce2e6b

GUKWAT changed the title ~~Codecleanup~~ Codecleanup #3 Jul 23, 2024

K-dash reviewed Jul 24, 2024

View reviewed changes

K-dash mentioned this pull request Jul 24, 2024

Improve makefile and pre-commit #56

Merged

GUKWAT added 7 commits July 27, 2024 09:36

test added for print location

5105fd6

changes made to the print_location function for the unit test to pass

2f8ecc7

test for set output values created

0066ff3

print_ocean_data test created

111aab0

print ocean data function modified

bf310f6

helper file formatted

acec795

formatted helper.py

02c421a

sourcery-ai bot reviewed Jul 29, 2024

View reviewed changes

GUKWAT added 3 commits August 2, 2024 00:37

docstrings modified for helper.py and test_helper.py

e3f6946

test created for print_location when show city is 0

8fbb0c2

print location function condition enhanced

b6bcce9

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Codecleanup #3 #54

Codecleanup #3 #54

GUKWAT commented Jul 23, 2024 •

edited by sourcery-ai bot

Loading

ryansurf commented Jul 23, 2024 •

edited

Loading

K-dash Jul 24, 2024 •

edited

Loading

GUKWAT Jul 24, 2024

GUKWAT Jul 24, 2024

ryansurf Jul 24, 2024

ryansurf Jul 25, 2024

GUKWAT Jul 28, 2024

ryansurf Jul 29, 2024

ryansurf commented Jul 29, 2024

sourcery-ai bot commented Jul 29, 2024 •

edited

Loading

sourcery-ai bot left a comment

sourcery-ai bot Jul 29, 2024

sourcery-ai bot Jul 29, 2024

sourcery-ai bot Jul 29, 2024

sourcery-ai bot Jul 29, 2024



		def print_ocean_data(arguments_dict, ocean_data_dict):
		def print_ocean_data(arguments_dict, ocean_data):

-def print_ocean_data(arguments_dict, ocean_data):
+def print_ocean_data(arguments_dict, ocean_data):
+    for key, value in ocean_data.items():
+        if value is None:
+            print(f"{key}: N/A")
+        else:
+            print(f"{key}: {value}")

Codecleanup #3 #54

Are you sure you want to change the base?

Codecleanup #3 #54

Conversation

GUKWAT commented Jul 23, 2024 • edited by sourcery-ai bot Loading

Summary by Sourcery

ryansurf commented Jul 23, 2024 • edited Loading

K-dash Jul 24, 2024 • edited Loading

Choose a reason for hiding this comment

GUKWAT Jul 24, 2024

Choose a reason for hiding this comment

GUKWAT Jul 24, 2024

Choose a reason for hiding this comment

ryansurf Jul 24, 2024

Choose a reason for hiding this comment

ryansurf Jul 25, 2024

Choose a reason for hiding this comment

GUKWAT Jul 28, 2024

Choose a reason for hiding this comment

ryansurf Jul 29, 2024

Choose a reason for hiding this comment

ryansurf commented Jul 29, 2024

sourcery-ai bot commented Jul 29, 2024 • edited Loading

Reviewer's Guide by Sourcery

File-Level Changes

sourcery-ai bot left a comment

Choose a reason for hiding this comment

sourcery-ai bot Jul 29, 2024

Choose a reason for hiding this comment

sourcery-ai bot Jul 29, 2024

Choose a reason for hiding this comment

sourcery-ai bot Jul 29, 2024

Choose a reason for hiding this comment

sourcery-ai bot Jul 29, 2024

Choose a reason for hiding this comment

GUKWAT commented Jul 23, 2024 •

edited by sourcery-ai bot

Loading

ryansurf commented Jul 23, 2024 •

edited

Loading

K-dash Jul 24, 2024 •

edited

Loading

sourcery-ai bot commented Jul 29, 2024 •

edited

Loading