Closed ryansurf closed 4 months ago
Coverage Report
File Stmts Miss Cover Missing src __init__.py 0 0 100% api.py 105 6 94% 30, 48, 70–71, 103–104 art.py 9 3 67% 24–25, 37 cli.py 25 5 80% 26, 37, 55–56, 60 dev_streamlit.py 37 37 0% 1–86 gpt.py 10 6 40% 16–21, 32–45 helper.py 159 60 62% 53, 55, 57, 59, 61, 63, 65, 67, 69, 71, 73, 75, 77, 79, 91, 102–106, 132, 134, 136, 145–155, 167, 180–181, 199–201, 211, 213–214, 237–238, 277–287, 294–302 send_email.py 24 24 0% 5–48 server.py 41 41 0% 5–82 settings.py 22 0 100% streamlit_helper.py 33 33 0% 5–90 TOTAL 465 215 54%
Tests | Skipped | Failures | Errors | Time |
---|---|---|---|---|
9 | 0 :zzz: | 0 :x: | 0 :fire: | 14.831s :stopwatch: |
I'm glad it seems to have worked well. Yeah, I think it's a good idea to adopt Streamlit! I'll list the things I think we should do in the future and the things that need to be considered, based on what comes to mind at this point.
Allow users to input query parameters that were being sent from the Curl command as Streamlit input fields
I agree. Wondering if we should let the user input something similar to the cli args, like show_wave,color=purple,gpt
etc. or we can use some toggles(like GPT=on/off)
Validation of input parameters (create a validation model in pydantic-settings)
Yep, its pretty easy to break the server at the moment
Visualization of weather forecast information
This would be the coolest feature imo. We have the pre-built map, but its not very useful. Something like this would be ideal (shows wind, wave height). I'm not sure how we would implement this ourselves
How to utilize GPT
I have an idea of how to do this. Simply have a on/off toggle for the GPT, and it its switched to "on" we can add gpt to the arguments, which is this line(64): surf_report = cli.run(args=["placeholder",f"{location}"])
-> surf_report = cli.run(args=["placeholder",f"{location}",gpt])
As for the name, I agree haha. Perhaps self-surf
, selfHosted-surf
or sh-surf
?
I agree. Wondering if we should let the user input something similar to the cli args, like show_wave,color=purple,gpt etc. or we can use some toggles(like GPT=on/off)
I have an idea of how to do this. Simply have a on/off toggle for the GPT, and it its switched to "on" we can add gpt to the arguments, which is this line(64): surf_report = cli.run(args=["placeholder",f"{location}"]) -> surf_report = cli.run(args=["placeholder",f"{location}",gpt])
I think it's good!
This would be the coolest feature imo. We have the pre-built map, but its not very useful. Something like this would be ideal (shows wind, wave height). I'm not sure how we would implement this ourselves
This seems like a great site that would be useful for surfers. It shows surf spots in Japan as well. I'm curious how they've accomplished that, but they likely have data on surf spots around the world. In any case, it would be difficult to implement similar functionality unless there is an API available to obtain information about surf spots...
As for the name, I agree haha. Perhaps self-surf, selfHosted-surf or sh-surf?
It's hard to decide on a name, isn't it? Haha. Maybe we can decide once the concept is more solidified!
@ryansurf Long time no see! It seems you haven't been active on Github lately. Are you busy? I was curious about the progress of the Streamlit implementation, so I decided to check in here😎
@ryansurf Long time no see! It seems you haven't been active on Github lately. Are you busy? I was curious about the progress of the Streamlit implementation, so I decided to check in here😎
@K-dash welcome back! Indeed, work got a little busy so I haven't been active on GitHub as much lately.
A few months ago when we were working more consistently on this project I submitted a PR to have the project be added to the up for grabs website. The PR was approved about a week ago and now some more people are finding out about our project! I'm dedicating more time to work on it going forward
@ryansurf Long time no see! It seems you haven't been active on Github lately. Are you busy? I was curious about the progress of the Streamlit implementation, so I decided to check in here😎
@K-dash welcome back! Indeed, work got a little busy so I haven't been active on GitHub as much lately.
A few months ago when we were working more consistently on this project I submitted a PR to have the project be added to the up for grabs website. The PR was approved about a week ago and now some more people are finding out about our project! I'm dedicating more time to work on it going forward
Good initiative! Please feel free to contact us if you need anything else:).
Hey @K-dash, I think we should merge this PR now.
The streamlit site is working well enough (definitely still needs some work), and I've updated the docs detailing how to run it.
At the moment I suppose we'll keep the other html/css frontend :flushed:
I've also made some slight updates to introduce the new make commands in the documentation, like make install
, make lint
and make format
.
Let me know what you think! This PR might seem large but its mostly just the streamlit implementation
Hey @K-dash, I think we should merge this PR now.
The streamlit site is working well enough (definitely still needs some work), and I've updated the docs detailing how to run it.
At the moment I suppose we'll keep the other html/css frontend 😳
I've also made some slight updates to introduce the new make commands in the documentation, like
make install
,make lint
andmake format
.Let me know what you think! This PR might seem large but its mostly just the streamlit implementation
I agree! I've commented on a few minor points. Thank you for merging the Makefile PR!
@K-dash , thanks for the review, I've implemented the fixes in https://github.com/ryansurf/cli-surf/pull/44/commits/103fdbb5f69c886a4432bd4823fb207277a33100.
I also resolved a merge conflict in https://github.com/ryansurf/cli-surf/pull/44/commits/77ab00bbd64e973f0404f8ed425940fd29e63422 and now I think we can merge the PR! Let me know what you think
@sourcery-ai review
This pull request introduces a Streamlit frontend for the surf forecasting application. It allows users to input a surf spot location and view a map of the spot along with a line graph displaying surf heights and swell periods. Additionally, it integrates GPT responses and provides options to toggle the map and GPT features. The changes also include refactoring and updates to helper functions, documentation, and tests.
Files | Changes |
---|---|
src/cli.py src/helper.py tests/test_cli.py |
Updated cli.run function to handle additional arguments and return GPT responses. Modified helper functions to support these changes and updated tests accordingly. |
README.md docs/cheat_sheet.md docs/structure.md docs/styling.md CONTRIBUTING.md |
Updated documentation to include instructions for the new Streamlit frontend and updated commands for running the linter and formatter. |
src/streamlit_helper.py src/dev_streamlit.py |
Implemented the Streamlit frontend and created helper functions to support it. |
@K-dash
Messed around with streamlit and got it working! (still a little rough)
It can now take a surf spot as input, and display a map that shows the spot along with a line graph of surf heights/swell periods
cli.py
, i added theargs=None
parameter. The reason being is that indev_streamlit.py
, we callcli.run
and want to specify a location in theargs
so that the function can find the lat/long of the location for us.dev_streamlit.py
, it now supports taking a location as input and outputting a map of said location along with a line chart displaying forecasted datahelper.py
, the json outputs are changed to be more concise(surf _height
->height
etc.)I think streamlit is what this project needed. This is also a draft PR, let me know what you think
example.webm
Edit: Updating this PR. In this update I enabled the user to use the GPT and added an option to hide the map
cli.py
, the function now returns thegpt response
as well as the ocean data. This way, the streamlit frontend can call thecli.run()
and obtain the gpt response to display (just printed the response prior to this)dev_streamlit.py
, the GPT and Map toggles have been addedstreamlit_helper.py
to store all the streamlit helper functions.helper.py
was getting a bit large/confusing imo.test_cli.py
, the test needs just the first item returned fromcli.run()
, which is why the index is used (it returns the data and gpt response now)Here is a demo of the new functionality:
demo.webm
Also, is it best practice for me to update the PR like I just did, or perhaps create another PR/branch? Unsure
Summary by Sourcery
This pull request introduces a new Streamlit frontend for the project, allowing users to input a surf spot and view a map and forecast data. It also adds GPT response functionality and options to toggle the map display. The codebase has been refactored for better organization, and documentation has been updated accordingly.
helper.py
to make JSON outputs more concise.streamlit_helper.py
to store all Streamlit helper functions, improving code organization.cli.py
to return both ocean data and GPT response for better integration with the Streamlit frontend.test_cli.py
to accommodate the new return structure ofcli.run()
.