cleaned up code in helper.py forecast_to_json()#159
cleaned up code in helper.py forecast_to_json()#159n-strong wants to merge 1 commit intoryansurf:mainfrom
Conversation
Reviewer's Guide by SourceryThe changes focus on improving the readability and efficiency of the No diagrams generated as the changes look simple and do not need a visual representation. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey @n-strong - I've reviewed your changes - here's some feedback:
Overall Comments:
- Please restore the removed docstring that describes the function's purpose and parameters. Documentation is important for maintainability.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
Hey @n-strong, sorry I haven't reviewed this yet, haven't had access to my laptop. I'll be able to this next week coming up🤙 looks good Saw you have some surf projects yourself, nice |
There was a problem hiding this comment.
Hey @n-strong, looks good! Looks cleaner now, especially the for loop. I left a comment asking about the removal of the docstring but everything else looks great.
Also, the linter failed. This should be an easy fix as its complaining about some whitespace. Checkout the makefile or refer to the docs here to see how to do this (just runingn make lint && make format should do the trick). Can you fix this?
Edit: Actually, I agree with removing the docstring. The function name is self-descriptive
|
|
||
|
|
||
| def forecast_to_json(forecast_data, decimal): | ||
| """ |
There was a problem hiding this comment.
Any reason you removed the docstring?
General:
Code:
I replaced range(len(...)) loops with enumerate() in forecast_to_json() and made the method more pythonic. Functionality remains the same.
Summary by Sourcery
Enhancements: