Skip to content
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

Make it possible to show rain volume #64

Merged
merged 3 commits into from
Nov 18, 2023
Merged

Conversation

cwitting
Copy link
Contributor

@cwitting cwitting commented Nov 8, 2023

This simply makes it possible to switch between showing Posibility of Precipitation (PoP) and rain volume in mm on the graph

Makes it possible to switch between showing
Posibility of Precipitation (PoP) and rain volume in mm
@lmarzen
Copy link
Owner

lmarzen commented Nov 8, 2023

I like this addition.
I have a few suggestions.
Some people may wish to change the unit. Particularly, I would advocate for an option to have the choice between mm, inches, and cm.
Second thing to note is that OpenWeatherMap has separate fields for rainfall and snowfall. I think we should add these fields so they are representative of the precipitation regardless of the season someone may be experiencing,

Let me know what you think?

@cwitting
Copy link
Contributor Author

cwitting commented Nov 9, 2023

Yes, I agree. I have added another commit which makes it possible to specify the unit, as well as combining snow and rain precipitation.

@lmarzen lmarzen self-requested a review November 9, 2023 16:36
@lmarzen
Copy link
Owner

lmarzen commented Nov 9, 2023

Looks good, I'll test it out this weekend and merge it.

@lmarzen
Copy link
Owner

lmarzen commented Nov 12, 2023

I have been busy with school work. I will try to get this merged over the thanksgiving holiday.

@lmarzen
Copy link
Owner

lmarzen commented Nov 18, 2023

Overall this is a great contribution. Thank you.
I added automatic precision adjustments for the y-axis ticks to reduce the likelihood that the rainfall will run off the screen.

@lmarzen lmarzen merged commit af580e6 into lmarzen:main Nov 18, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants