Day summary/icon incorrect

cloneofghosts commented 2 weeks ago

Describe the bug

This morning I noticed a few instances where the day icon and summary aren't being calculated correctly. Examples below (cut out any details that weren't relevant in the day block):

Today

  "icon": "clear-day",
  "summary": "Clear",
  "precipIntensity": 0,
  "precipIntensityMax": 0,
  "precipProbability": 0.05,
  "precipAccumulation": 0,
  "cloudCover": 0.4,

Tuesday

  "icon": "clear-day",
  "summary": "Clear",
  "precipIntensity": 0.0161,
  "precipIntensityMax": 0.0468,
  "precipProbability": 0.44,
  "precipAccumulation": 0,
  "cloudCover": 0.38,

Wednesday

  "icon": "partly-cloudy-day",
  "summary": "Partly Cloudy",
  "precipIntensity": 0.0326,
  "precipIntensityMax": 0.2587,
  "precipProbability": 0.22,
  "precipAccumulation": 0.0698,
  "cloudCover": 0.37,

Expected behavior

Today and Tuesday should show Partly Cloudy as the summary and icon. Wednesday should show Clear as the summary and icon.

Actual behavior

Today and Tuesday shows Clear as the summary and icon. Wednesday shows Partly Cloudy as the summary and icon.

API Endpoint

Production

Location

Barrhaven, Ontario

Other details

This isn't related to the icon issue but I noticed that Tuesday has a precipIntensity above zero whereas the precipAccumulation is 0. I thought it would be above zero in this case.

Troubleshooting steps

[X] I have searched this repository and Home Assistant Repository to see if the issue has already been reported.
[X] I have read through the API documentation before opening this issue.
[X] I have not included my API key anywhere in my issue.
[X] I have written an informative title.

cloneofghosts commented 2 weeks ago

@alexander0042 Still seeing issues with this the next day. Also noticed one day that should show the rain icon but isn't for some reason. Also accumulation seems to have way more than four decimal points for some reason.

Tuesday

  "icon": "clear-day",
  "summary": "Clear",
  "precipIntensity": 0.0373,
  "precipIntensityMax": 0.2392,
  "precipProbability": 0.78,
  "precipAccumulation": 0.21650000000000003,
  "precipType": "rain",
  "cloudCover": 0.33,

Wednesday

  "icon": "partly-cloudy-day",
  "summary": "Partly Cloudy",
  "precipIntensity": 0,
  "precipIntensityMax": 0,
  "precipProbability": 0.09,
  "precipAccumulation": 0,
  "precipType": "snow",
  "cloudCover": 0.32,

cloneofghosts commented 2 weeks ago

@alexander0042 Sorry for the repeated pings on here but this is still an issue. A bit annoying to have to double check the daily details in case the icon/summary shown is incorrect.

alexander0042 commented 1 week ago

I'm 90% sure I've isolated what's happening here! The icon is calculated from the 4 am to 4 am times; however, the averaged values (which is what you're seeing for cloud cover and the like) are calculated from 00:00 to 00:00. So while they align almost all of the time, they can be off.

I think the easiest fix here is just to change everything to the 4:00 reset. I can't really imagine a use where someone would want the 00 times, and that data could always be calculated from hourly if need be

cloneofghosts commented 5 days ago

@alexander0042 Is this something that will be fixed for V2.4? Not a huge deal if not, I'll just remove it from the PR so it doesn't close this when it gets merged in.

alexander0042 commented 1 day ago

Ok, I mulled on this one over the weekend and I'm honestly not sure about this one. The obvious fix would be to switch everything over to the 4am/pm times, which was my initial though, but it just doesn't feel right on some level. If someone is pulling the data, it's pretty intuitive to expect that "daily" is from 0 to 24, particularly for things like accumulations. So along these lines, I don't know if I like moving everything over to that.

I'm thinking maybe this is something that we can address with the improved text summary- maybe make the daily icons based on 0 to 24 to align with the data, but keep the text 4 to 4? I really don't have a perfect solution here, so open to suggestions!

cloneofghosts commented 1 day ago

Yeah, I'm not entirely sure how exactly Dark Sky did it. I was looking in the Wayback Machine yesterday when I was checking to see how they did their Dry/Humid text and did notice multiple instances of the summary and icon being different (rain icon but summary showed Partly cloudy throughout the day).

Having the text summaries and #49 would definitely help when they get added though it may not be for a while. Not sure what a good short-term solution would be. I did have a thought about adding fields for this but I'm not sure we need more fields similar to the min/max and high/low temperature fields.

alexander0042 commented 1 day ago

For now, I think we just leave things as we are and release 2.4.0 as is. This is a bit of an edge case, so not a super high priority, but very much worth keeping in mind

cloneofghosts commented 1 day ago

Alright, sounds good. Will close this for now and we can revisit then the text summaries get implemented.

Pirate-Weather / pirateweather