Add more data #111

MylesPerHour201 · 2024-08-11T20:43:26Z

Fixes #4

Created get_hourly_forecast function to get hourly weather data
Updated gather_data function to take in the get_hourly_forecast function
Added "Cloud Cover" and "Visibility" to ocean_data_dict
Added "show_visibility" and "show_cloud_cover" to arguments dictionary
Removed "city" as a parameter of print_outputs function to pass linter, as "city" is called in the arguments dictionary and does not need to be in parameters of print_outputs
Created print_hourly_data function to print the output of the get_hourly_forecast function
Added print_hourly_data function to print_outputs to include the hourly data in the main printing function
Added curr_hour_data to cli.py to print hourly weather data

Summary by Sourcery

Add functionality to retrieve and display hourly weather data, including cloud cover and visibility. Enhance existing data gathering and output functions to incorporate the new hourly data. Fix linter issue by removing unnecessary 'city' parameter from print_outputs function.

New Features:

Added get_hourly_forecast function to retrieve hourly weather data including cloud cover and visibility.
Introduced print_hourly_data function to display hourly weather data.

Bug Fixes:

Removed 'city' parameter from print_outputs function to resolve linter issues.

Enhancements:

Updated gather_data function to incorporate hourly weather data.
Modified print_outputs function to include hourly weather data in the main output.

sourcery-ai · 2024-08-11T20:43:33Z

Reviewer's Guide by Sourcery

This pull request adds functionality to fetch and print hourly weather data, including cloud cover and visibility. The get_hourly_forecast function was created to fetch the data, and the gather_data function was updated to include this data in the ocean_data_dict. The print_outputs function was modified to call a new print_hourly_data function, and the CLI was updated to handle the new data.

File-Level Changes

Files	Changes
`src/api.py` `src/helper.py` `src/cli.py`	Introduced functionality to fetch and print hourly weather data, including cloud cover and visibility.

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 @MylesPerHour201 - I've reviewed your changes - here's some feedback:

Overall Comments:

Consider making the new 'show_visibility' and 'show_cloud_cover' arguments in the arguments_dictionary function configurable rather than hard-coding them to 1.
The addition of hourly forecast data is a good feature, but consider the potential impact on API usage and performance. You might want to implement caching for this data to reduce API calls.

Here's what I looked at during the review

🟡 General issues: 3 issues found
🟢 Security: all looks good
🟢 Testing: all looks good
🟢 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-08-11T20:44:48Z

src/api.py

@@ -294,6 +295,62 @@ def forecast(lat, long, decimal, days=0):
    return forecast_data


+def get_hourly_forecast(lat, long, days=1):


suggestion: Consider adding error handling for API calls and data processing

The function currently doesn't handle potential exceptions from API calls or data processing. Adding try-except blocks would make the function more robust and prevent unexpected crashes.

def get_hourly_forecast(lat, long, days=1): try: # API call and data processing here pass except requests.RequestException as e: logging.error(f"API request failed: {e}") return None except ValueError as e: logging.error(f"Data processing error: {e}") return None

sourcery-ai · 2024-08-11T20:44:49Z

src/helper.py

    """
    Basically the main printing function,
    calls all the other printing functions
    """
    print("\n")
-    if city == "No data":
-        print("No location found")
+    # if city == "No data":


suggestion: Remove commented-out code

If this code is no longer needed, it should be removed entirely rather than left commented out. This improves code readability and maintainability.

Suggested change

# if city == "No data":

if ocean_data_dict["Height"] == "No data":

sourcery-ai · 2024-08-11T20:44:49Z

src/cli.py

@@ -42,11 +42,14 @@ def run(lat=0, long=0, args=None):
    # Makes calls to the apis(ocean, UV) and returns the values in a dictionary
    ocean_data_dict = api.gather_data(lat, long, arguments)

+    # Sets hourly_dict = the dictionary for hourly weather data
+    curr_hour_data = api.get_hourly_forecast(lat, long, days=1)


suggestion: Maintain consistent variable naming

The variable 'curr_hour_data' is set here, but it's passed as 'curr_hour' to print_outputs. Consider using consistent naming to improve code clarity.

hourly_forecast = api.get_hourly_forecast(lat, long, days=1)

ryansurf · 2024-08-12T18:46:54Z

Hey @MylesPerHour201

Nice work. I'm going to review your PR later today

In the meantime, can you fix the linter issues? The test failed

ryansurf

Look great! I left a couple comments on some changes I think would be necessary

Also, the readme should be updated to reflect the new arguments (like show_cloud_cover/scc or show_vis/sv)

Let me know if this all makes sense

ryansurf · 2024-08-15T04:40:29Z

src/helper.py

@@ -40,6 +40,8 @@ def arguments_dictionary(lat, long, city, args):
        "forecast_days": get_forecast_days(args),
        "color": get_color(args),
        "gpt": 0,
+        "show_visibility": 1,
+        "show_cloud_cover": 1,


We should set these to 0 as default, and show them if the user specifies in a cli argument (like show_vis or sv)

ryansurf · 2024-08-15T04:44:09Z

src/api.py

+        "latitude": lat,
+        "longitude": long,
+        "hourly": ["cloud_cover", "visibility"],
+        "temperature_unit": "fahrenheit",


It would be nice to pass in an argument that specifies the temperature unit. We can default to Fahrenheit. If the user passes the metric argument, Celsius should be shown.

See the ocean_information function to get an idea

ryansurf · 2024-08-15T04:47:39Z

src/api.py

@@ -312,6 +368,8 @@ def gather_data(lat, long, arguments):

    wind_temp = current_wind_temp(lat, long, arguments["decimal"])

+    hourly_dict = get_hourly_forecast(lat, long, arguments["decimal"])


You call get_hourly_forecast here and also in cli.py. It should only be called here, in gather_data

ryansurf · 2024-08-15T04:48:23Z

src/cli.py

@@ -42,11 +42,14 @@ def run(lat=0, long=0, args=None):
    # Makes calls to the apis(ocean, UV) and returns the values in a dictionary
    ocean_data_dict = api.gather_data(lat, long, arguments)

+    # Sets hourly_dict = the dictionary for hourly weather data
+    hourly_forecast = api.get_hourly_forecast(lat, long, days=1)
+


We call api.get_hourly_forecast already in api.gather_data() above

ryansurf · 2024-08-15T04:49:37Z

src/cli.py

    # Non-JSON output
    if arguments["json_output"] == 0:
        # Response prints all the outputs & returns the GPT response
        response = helper.print_outputs(
-            city, ocean_data_dict, arguments, gpt_prompt, gpt_info
+            hourly_forecast, ocean_data_dict, arguments, gpt_prompt, gpt_info


the ocean_data_dict should have all the hourly forecast data, if im not mistaken. It doesn't seem necessary to pass it as an argument

ryansurf · 2024-08-15T16:24:40Z

src/helper.py

@@ -379,3 +386,16 @@ def print_gpt(surf_data, gpt_prompt, gpt_info):
    else:
        gpt_response = gpt.openai_gpt(summary, gpt_prompt, api_key, gpt_model)
    return gpt_response
+
+
+def print_hourly_data(curr_hour):


Is this function ever used? I commented it out when testing and everything (as far as I can tell) still functions properly.

No, I don't think it is now with the newest changes based on your suggestions. Would you like me to remove it?

Yes please, once thats done I'll merge the PR

ryansurf · 2024-08-15T16:42:49Z

@all-contributors please add @MylesPerHour201 for code

allcontributors · 2024-08-15T16:42:59Z

@ryansurf

I've put up a pull request to add @MylesPerHour201! 🎉

created functions to get and print hourly weather

775afc3

sourcery-ai bot reviewed Aug 11, 2024

View reviewed changes

updated cli.py helper.py based on sourcery suggestions

7104c6a

MylesPerHour201 and others added 2 commits August 12, 2024 18:34

updated api.py to pass linter

3080ef8

Merge branch 'ryansurf:main' into main

0c0b297

ryansurf reviewed Aug 15, 2024

View reviewed changes

made necessary edits highlighted in PR review

b52fc17

ryansurf reviewed Aug 15, 2024

View reviewed changes

removed print_hourly_data function

ef49210

allcontributors bot mentioned this pull request Aug 15, 2024

docs: add MylesPerHour201 as a contributor for code #123

Merged

ryansurf merged commit 03cab41 into ryansurf:main Aug 15, 2024
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add more data #111

Add more data #111

MylesPerHour201 commented Aug 11, 2024 •

edited by sourcery-ai bot

Loading

sourcery-ai bot commented Aug 11, 2024 •

edited

Loading

sourcery-ai bot left a comment

sourcery-ai bot Aug 11, 2024

sourcery-ai bot Aug 11, 2024

sourcery-ai bot Aug 11, 2024

ryansurf commented Aug 12, 2024 •

edited

Loading

ryansurf left a comment

ryansurf Aug 15, 2024

ryansurf Aug 15, 2024

ryansurf Aug 15, 2024

ryansurf Aug 15, 2024

ryansurf Aug 15, 2024

ryansurf Aug 15, 2024

MylesPerHour201 Aug 15, 2024

ryansurf Aug 15, 2024

ryansurf commented Aug 15, 2024

allcontributors bot commented Aug 15, 2024

		@@ -294,6 +295,62 @@ def forecast(lat, long, decimal, days=0):
		return forecast_data


		def get_hourly_forecast(lat, long, days=1):

	# if city == "No data":
	if ocean_data_dict["Height"] == "No data":

		@@ -312,6 +368,8 @@ def gather_data(lat, long, arguments):

		wind_temp = current_wind_temp(lat, long, arguments["decimal"])

		hourly_dict = get_hourly_forecast(lat, long, arguments["decimal"])

Add more data #111

Add more data #111

Conversation

MylesPerHour201 commented Aug 11, 2024 • edited by sourcery-ai bot Loading

Summary by Sourcery

sourcery-ai bot commented Aug 11, 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 Aug 11, 2024

Choose a reason for hiding this comment

sourcery-ai bot Aug 11, 2024

Choose a reason for hiding this comment

sourcery-ai bot Aug 11, 2024

Choose a reason for hiding this comment

ryansurf commented Aug 12, 2024 • edited Loading

ryansurf left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ryansurf commented Aug 15, 2024

allcontributors bot commented Aug 15, 2024

MylesPerHour201 commented Aug 11, 2024 •

edited by sourcery-ai bot

Loading

sourcery-ai bot commented Aug 11, 2024 •

edited

Loading

ryansurf commented Aug 12, 2024 •

edited

Loading